From 0229807f3681055f22526f2015dc457afad4d3b6 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Fri, 30 Aug 2024 14:16:00 +1000 Subject: [PATCH 01/17] Move common types to superdesk.core.types --- superdesk/core/elastic/async_client.py | 2 +- superdesk/core/elastic/base_client.py | 15 +++-- superdesk/core/elastic/common.py | 2 +- superdesk/core/elastic/sync_client.py | 2 +- superdesk/core/resources/cursor.py | 67 +-------------------- superdesk/core/types.py | 82 ++++++++++++++++++++++++++ tests/core/elastic_async_test.py | 2 +- tests/core/elastic_sync_test.py | 2 +- tests/core/resource_service_test.py | 2 +- 9 files changed, 98 insertions(+), 78 deletions(-) create mode 100644 superdesk/core/types.py diff --git a/superdesk/core/elastic/async_client.py b/superdesk/core/elastic/async_client.py index 033918edc3..641f784561 100644 --- a/superdesk/core/elastic/async_client.py +++ b/superdesk/core/elastic/async_client.py @@ -14,7 +14,7 @@ from elasticsearch.exceptions import NotFoundError, TransportError, RequestError from elasticsearch.helpers import async_bulk -from ..resources.cursor import SearchRequest +from superdesk.core.types import SearchRequest from .base_client import BaseElasticResourceClient, ElasticCursor, InvalidSearchString diff --git a/superdesk/core/elastic/base_client.py b/superdesk/core/elastic/base_client.py index 3cd787c85f..d664097da8 100644 --- a/superdesk/core/elastic/base_client.py +++ b/superdesk/core/elastic/base_client.py @@ -8,14 +8,14 @@ # AUTHORS and LICENSE files distributed with this source code, or # at https://www.sourcefabric.org/superdesk/license -from typing import Dict, Any, Optional, Iterator, List, Tuple, Union, TypedDict +from typing import Dict, Any, Optional, Iterator, List, Tuple, Union, TypedDict, Literal import ast import simplejson as json from eve.io.mongo.parser import parse from superdesk.errors import SuperdeskApiError -from ..resources.cursor import ProjectedFieldArg, SearchRequest +from superdesk.core.types import ProjectedFieldArg, SearchRequest, SortParam from .common import ElasticClientConfig, ElasticResourceConfig @@ -172,8 +172,7 @@ def _get_find_args( if "sort" not in query: if req.sort: - sort = ast.literal_eval(req.sort) - _set_sort(query, sort) + _set_sort(query, req.sort) elif self.resource_config.default_sort: _set_sort(query, self.resource_config.default_sort) @@ -282,9 +281,13 @@ def _format_doc(self, hit: Dict[str, Any]): return doc -def _set_sort(query, sort): +def _set_sort(query: dict, sort: SortParam | None) -> None: + if sort is None: + return + query["sort"] = [] - for key, sortdir in sort: + sort_list = ast.literal_eval(sort) if isinstance(sort, str) else sort + for key, sortdir in sort_list: sort_dict = dict([(key, "asc" if sortdir > 0 else "desc")]) query["sort"].append(sort_dict) diff --git a/superdesk/core/elastic/common.py b/superdesk/core/elastic/common.py index 95295bf6ce..fe67512433 100644 --- a/superdesk/core/elastic/common.py +++ b/superdesk/core/elastic/common.py @@ -14,7 +14,7 @@ from uuid import uuid4 from ..config import ConfigModel -from ..resources.cursor import SearchRequest +from superdesk.core.types import SearchRequest @dataclass diff --git a/superdesk/core/elastic/sync_client.py b/superdesk/core/elastic/sync_client.py index 6dfc110a5d..b49cc89311 100644 --- a/superdesk/core/elastic/sync_client.py +++ b/superdesk/core/elastic/sync_client.py @@ -14,7 +14,7 @@ from elasticsearch.exceptions import NotFoundError, TransportError, RequestError from elasticsearch.helpers import bulk -from ..resources.cursor import SearchRequest +from superdesk.core.types import SearchRequest from .base_client import BaseElasticResourceClient, ElasticCursor, InvalidSearchString diff --git a/superdesk/core/resources/cursor.py b/superdesk/core/resources/cursor.py index 7d894997ee..bd2c22ad7e 100644 --- a/superdesk/core/resources/cursor.py +++ b/superdesk/core/resources/cursor.py @@ -8,76 +8,11 @@ # AUTHORS and LICENSE files distributed with this source code, or # at https://www.sourcefabric.org/superdesk/license -from typing import Dict, Any, Generic, TypeVar, Type, Optional, List, Union, Literal -from typing_extensions import TypedDict +from typing import Dict, Any, Generic, TypeVar, Type, Optional, List -from pydantic import BaseModel, ConfigDict from motor.motor_asyncio import AsyncIOMotorCollection, AsyncIOMotorCursor -#: The data type for projections, either a list of field names, or a dictionary containing -#: the field and enable/disable state -ProjectedFieldArg = Union[List[str], Dict[str, Literal[0]], Dict[str, Literal[1]]] - - -class SearchArgs(TypedDict, total=False): - """Dictionary containing Elasticsearch search arguments - - This is for use with the `.find` methods in elastic clients - """ - - #: A JSON string containing an elasticsearch query - source: str - - #: A query string - q: str - - #: Default field, for use with the query string - df: str - - #: Default operator, for use with the query string (defaults to "AND") - default_operator: str - - #: A JSON string containing bool query filters, to be applied to the elastic query - filter: str - - #: A list of dictionaries containing bool query filters, to be applied to the elastic query - filters: List[Dict[str, Any]] - - #: A JSON string containing the field projections to filter out the returned fields - projections: str - - -class SearchRequest(BaseModel): - """Dataclass containing Elasticsearch request arguments""" - - model_config = ConfigDict(extra="allow") - - #: Argument for the search filters - args: Optional[SearchArgs] = None - - #: Sorting to be used - sort: Optional[str] = None - - #: Maximum number of documents to be returned - max_results: int = 25 - - #: The page number to be returned - page: int = 1 - - #: A JSON string containing an Elasticsearch where query - where: Optional[Union[str, Dict]] = None - - #: If `True`, will include aggregations with the result - aggregations: bool = False - - #: If `True`, will include highlights with the result - highlight: bool = False - - #: The field projections to be applied - projection: Optional[ProjectedFieldArg] = None - - ResourceModelType = TypeVar("ResourceModelType", bound="ResourceModel") diff --git a/superdesk/core/types.py b/superdesk/core/types.py new file mode 100644 index 0000000000..fb68bb60bc --- /dev/null +++ b/superdesk/core/types.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8; -*- +# +# This file is part of Superdesk. +# +# Copyright 2024 Sourcefabric z.u. and contributors. +# +# For the full copyright and license information, please see the +# AUTHORS and LICENSE files distributed with this source code, or +# at https://www.sourcefabric.org/superdesk/license + +from typing import Dict, Any, Optional, List, Union, Literal +from typing_extensions import TypedDict + +from pydantic import BaseModel, ConfigDict, NonNegativeInt + + +#: The data type for projections, either a list of field names, or a dictionary containing +#: the field and enable/disable state +ProjectedFieldArg = Union[List[str], Dict[str, Literal[0]], Dict[str, Literal[1]]] +SortListParam = list[tuple[str, Literal[1, -1]]] +SortParam = str | SortListParam +VersionParam = Literal["all"] | NonNegativeInt + + +class SearchArgs(TypedDict, total=False): + """Dictionary containing Elasticsearch search arguments + + This is for use with the `.find` methods in elastic clients + """ + + #: A JSON string containing an elasticsearch query + source: str + + #: A query string + q: str + + #: Default field, for use with the query string + df: str + + #: Default operator, for use with the query string (defaults to "AND") + default_operator: str + + #: A JSON string containing bool query filters, to be applied to the elastic query + filter: str + + #: A list of dictionaries containing bool query filters, to be applied to the elastic query + filters: List[Dict[str, Any]] + + #: A JSON string containing the field projections to filter out the returned fields + projections: str + + version: VersionParam | None + + +class SearchRequest(BaseModel): + """Dataclass containing Elasticsearch request arguments""" + + model_config = ConfigDict(extra="allow") + + #: Argument for the search filters + args: Optional[SearchArgs] = None + + #: Sorting to be used + sort: SortParam | None = None + + #: Maximum number of documents to be returned + max_results: int = 25 + + #: The page number to be returned + page: int = 1 + + #: A JSON string contianing an Elasticsearch where query + where: str | dict | None = None + + #: If `True`, will include aggregations with the result + aggregations: bool = False + + #: If `True`, will include highlights with the result + highlight: bool = False + + #: The field projections to be applied + projection: Optional[ProjectedFieldArg] = None diff --git a/tests/core/elastic_async_test.py b/tests/core/elastic_async_test.py index 4060c2898b..6601af5428 100644 --- a/tests/core/elastic_async_test.py +++ b/tests/core/elastic_async_test.py @@ -1,7 +1,7 @@ import simplejson as json from superdesk.tests import AsyncTestCase -from superdesk.core.resources.cursor import SearchRequest +from superdesk.core.types import SearchRequest from .modules.users import User from .fixtures.users import john_doe diff --git a/tests/core/elastic_sync_test.py b/tests/core/elastic_sync_test.py index 804444bb39..7c2c21d706 100644 --- a/tests/core/elastic_sync_test.py +++ b/tests/core/elastic_sync_test.py @@ -1,7 +1,7 @@ import simplejson as json from superdesk.tests import AsyncTestCase -from superdesk.core.resources.cursor import SearchRequest +from superdesk.core.types import SearchRequest from .modules.users import User from .fixtures.users import john_doe diff --git a/tests/core/resource_service_test.py b/tests/core/resource_service_test.py index 70987e4349..d7ccf1068e 100644 --- a/tests/core/resource_service_test.py +++ b/tests/core/resource_service_test.py @@ -4,7 +4,7 @@ import simplejson as json from bson import ObjectId -from superdesk.core.resources.cursor import SearchRequest +from superdesk.core.types import SearchRequest from superdesk.utc import utcnow from superdesk.utils import format_time From f10bd0792e4f27fc2b96b749ccecc475dcc28450 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 4 Sep 2024 15:25:59 +1000 Subject: [PATCH 02/17] support default sort resource config --- superdesk/core/elastic/common.py | 4 ++-- superdesk/core/resources/model.py | 6 ++++++ superdesk/core/resources/resource_rest_endpoints.py | 2 +- superdesk/core/types.py | 8 ++++++++ superdesk/core/web/rest_endpoints.py | 2 +- 5 files changed, 18 insertions(+), 4 deletions(-) diff --git a/superdesk/core/elastic/common.py b/superdesk/core/elastic/common.py index fe67512433..b37f489e5f 100644 --- a/superdesk/core/elastic/common.py +++ b/superdesk/core/elastic/common.py @@ -14,7 +14,7 @@ from uuid import uuid4 from ..config import ConfigModel -from superdesk.core.types import SearchRequest +from superdesk.core.types import SearchRequest, SortParam @dataclass @@ -25,7 +25,7 @@ class ElasticResourceConfig: prefix: str = "ELASTICSEARCH" #: The default sort - default_sort: Optional[str] = None + default_sort: SortParam | None = None #: The default maximum number of documents to be returned default_max_results: Optional[int] = None diff --git a/superdesk/core/resources/model.py b/superdesk/core/resources/model.py index fdc5d3a0af..a6580b3fb7 100644 --- a/superdesk/core/resources/model.py +++ b/superdesk/core/resources/model.py @@ -28,6 +28,7 @@ from pydantic_core import InitErrorDetails, PydanticCustomError from pydantic.dataclasses import dataclass as pydataclass +from superdesk.core.types import SortListParam from .fields import ObjectId @@ -201,6 +202,9 @@ class ResourceConfig: #: Optional list of resource fields to ignore when generating the etag etag_ignore_fields: Optional[list[str]] = None + #: Optional sorting for this resource + default_sort: SortListParam | None = None + class Resources: """A high level resource class used to manage all resources in the system""" @@ -239,6 +243,8 @@ def register(self, config: ResourceConfig): ) if config.elastic is not None: + if config.default_sort: + config.elastic.default_sort = config.default_sort self.app.elastic.register_resource_config( config.name, config.elastic, diff --git a/superdesk/core/resources/resource_rest_endpoints.py b/superdesk/core/resources/resource_rest_endpoints.py index 9fc5fc0f01..ad1b94044f 100644 --- a/superdesk/core/resources/resource_rest_endpoints.py +++ b/superdesk/core/resources/resource_rest_endpoints.py @@ -18,12 +18,12 @@ from superdesk.core.app import get_current_async_app from superdesk.errors import SuperdeskApiError +from superdesk.core.types import SearchRequest, SearchArgs from ..web.types import HTTP_METHOD, Request, Response, RestGetResponse from ..web.rest_endpoints import RestEndpoints, ItemRequestViewArgs from .model import ResourceConfig, ResourceModel -from .cursor import SearchRequest, SearchArgs from .validators import convert_pydantic_validation_error_for_response from .utils import resource_uses_objectid_for_id diff --git a/superdesk/core/types.py b/superdesk/core/types.py index fb68bb60bc..73ad09fe24 100644 --- a/superdesk/core/types.py +++ b/superdesk/core/types.py @@ -17,8 +17,16 @@ #: The data type for projections, either a list of field names, or a dictionary containing #: the field and enable/disable state ProjectedFieldArg = Union[List[str], Dict[str, Literal[0]], Dict[str, Literal[1]]] + +#: Type used to provide list of sort params to be used SortListParam = list[tuple[str, Literal[1, -1]]] + +#: Type used for sort param in service requests +#: can be a string, which will convert to an :attr:`SortListParam` type SortParam = str | SortListParam + +#: Type used for version param in service requests +#: Can be either ``"all"`` or an int ``>= 0`` VersionParam = Literal["all"] | NonNegativeInt diff --git a/superdesk/core/web/rest_endpoints.py b/superdesk/core/web/rest_endpoints.py index c925473bc4..6bdb4e306e 100644 --- a/superdesk/core/web/rest_endpoints.py +++ b/superdesk/core/web/rest_endpoints.py @@ -12,8 +12,8 @@ from pydantic import BaseModel +from superdesk.core.types import SearchRequest from .types import Endpoint, EndpointGroup, HTTP_METHOD, Request, Response -from ..resources.cursor import SearchRequest class ItemRequestViewArgs(BaseModel): From 1269fadbdd2375fb72bd74d1da508b508f87e0f8 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 4 Sep 2024 15:26:08 +1000 Subject: [PATCH 03/17] improve: add `to_list` to ResourceCursorAsync --- superdesk/core/resources/cursor.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/superdesk/core/resources/cursor.py b/superdesk/core/resources/cursor.py index bd2c22ad7e..fbbafa5b5b 100644 --- a/superdesk/core/resources/cursor.py +++ b/superdesk/core/resources/cursor.py @@ -29,6 +29,14 @@ async def __anext__(self) -> ResourceModelType: async def next_raw(self) -> Optional[Dict[str, Any]]: raise NotImplementedError() + async def to_list(self) -> List[ResourceModelType]: + items: List[ResourceModelType] = [] + item = await self.next_raw() + while item is not None: + items.append(self.get_model_instance(item)) + item = await self.next_raw() + return items + async def to_list_raw(self) -> List[Dict[str, Any]]: items: List[Dict[str, Any]] = [] item = await self.next_raw() From 559edf5724a94fb16d05eff39521b19e736e369b Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 4 Sep 2024 15:26:17 +1000 Subject: [PATCH 04/17] improve: allow kwargs to AsyncResourceService.find --- superdesk/core/resources/service.py | 45 ++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/superdesk/core/resources/service.py b/superdesk/core/resources/service.py index 7a739d9622..dbd1947e26 100644 --- a/superdesk/core/resources/service.py +++ b/superdesk/core/resources/service.py @@ -21,6 +21,7 @@ cast, Tuple, Literal, + overload, ) import logging import ast @@ -35,10 +36,11 @@ from superdesk.errors import SuperdeskApiError from superdesk.utc import utcnow from superdesk.json_utils import SuperdeskJSONEncoder +from superdesk.core.types import SearchRequest, SortListParam, SortParam from ..app import SuperdeskAsyncApp, get_current_async_app from .fields import ObjectId as ObjectIdField -from .cursor import ElasticsearchResourceCursorAsync, MongoResourceCursorAsync, ResourceCursorAsync, SearchRequest +from .cursor import ElasticsearchResourceCursorAsync, MongoResourceCursorAsync, ResourceCursorAsync from .utils import resource_uses_objectid_for_id logger = logging.getLogger(__name__) @@ -414,19 +416,52 @@ async def get_all_batch(self, size=500, max_iterations=10000, lookup=None) -> As else: logger.warning(f"Not enough iterations for resource {self.resource_name}") + @overload async def find(self, req: SearchRequest) -> ResourceCursorAsync[ResourceModelType]: + ... + + @overload + async def find( + self, req: dict, page: int = 1, max_results: int = 25, sort: SortParam | None = None + ) -> ResourceCursorAsync[ResourceModelType]: + ... + + async def find( + self, + req: SearchRequest | dict, + page: int = 1, + max_results: int = 25, + sort: SortParam | None = None, + ) -> ResourceCursorAsync[ResourceModelType]: """Find items from the resource using Elasticsearch - :param req: A SearchRequest instance with the search params to be used + :param req: SearchRequest instance, or a lookup dictionary, for the search params to be used + :param page: The page number to retrieve (defaults to 1) + :param max_results: The maximum number of results to retrieve per page (defaults to 25) + :param sort: The sort order to use (defaults to resource default sort, or not sorting applied) :return: An async iterable with ``ResourceModel`` instances :raises SuperdeskApiError.notFoundError: If Elasticsearch is not configured """ + search_request = ( + req + if isinstance(req, SearchRequest) + else SearchRequest( + where=req if req else None, + page=page, + max_results=max_results, + sort=sort, + ) + ) + + if search_request.sort is None: + search_request.sort = self.config.default_sort + try: - cursor, count = await self.elastic.find(req) + cursor, count = await self.elastic.find(search_request) return ElasticsearchResourceCursorAsync(self.config.data_class, cursor.hits) except KeyError: - return await self._mongo_find(req) + return await self._mongo_find(search_request) async def _mongo_find(self, req: SearchRequest) -> MongoResourceCursorAsync: args: Dict[str, Any] = {} @@ -450,6 +485,8 @@ async def _mongo_find(self, req: SearchRequest) -> MongoResourceCursorAsync: def _convert_req_to_mongo_sort(self, req: SearchRequest) -> List[Tuple[str, Literal[1, -1]]]: if not req.sort: return [] + elif isinstance(req.sort, list): + return req.sort client_sort: List[Tuple[str, Literal[1, -1]]] = [] try: From 8b4b475893c8b942a07f33e855ad5287a806b625 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 4 Sep 2024 15:26:27 +1000 Subject: [PATCH 05/17] improve: support web endpoint with empty args --- superdesk/core/web/types.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/superdesk/core/web/types.py b/superdesk/core/web/types.py index 52e081d146..bcc5da5e46 100644 --- a/superdesk/core/web/types.py +++ b/superdesk/core/web/types.py @@ -53,6 +53,9 @@ class Response: #: #: Supported endpoint signatures:: #: +#: # Response only +#: async def test() -> Response: +#: #: # Request Only #: async def test1(request: Request) -> Response #: @@ -77,6 +80,10 @@ class Response: #: request: Request #: ) -> Response EndpointFunction = Union[ + Callable[ + [], + Awaitable[Response], + ], Callable[ ["Request"], Awaitable[Response], @@ -129,7 +136,9 @@ def __init__( def __call__(self, args: Dict[str, Any], params: Dict[str, Any], request: "Request"): func_params = signature(self.func).parameters - if "args" not in func_params and "params" not in func_params: + if not len(func_params): + return self.func() + elif "args" not in func_params and "params" not in func_params: return self.func(request) # type: ignore[call-arg,arg-type] arg_type = func_params["args"] if "args" in func_params else None From 2a67144235b6d16c40f6ad64c8e662b0d192d63a Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 4 Sep 2024 15:26:36 +1000 Subject: [PATCH 06/17] add/fix tests --- superdesk/tests/__init__.py | 3 ++ tests/core/modules/users/types.py | 6 +-- tests/core/resource_model_test.py | 6 +-- tests/core/resource_service_test.py | 72 +++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/superdesk/tests/__init__.py b/superdesk/tests/__init__.py index 54b7f816b6..da9b2b8435 100644 --- a/superdesk/tests/__init__.py +++ b/superdesk/tests/__init__.py @@ -565,6 +565,9 @@ def get_fixture_path(self, filename): rootpath = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) return os.path.join(rootpath, "features", "steps", "fixtures", filename) + def assertDictContains(self, source: dict, contains: dict): + self.assertDictEqual({key: val for key, val in source.items() if key in contains}, contains) + class TestClient(QuartClient): def model_instance_to_json(self, model_instance: ResourceModel): diff --git a/tests/core/modules/users/types.py b/tests/core/modules/users/types.py index 130569411f..dacffaf37b 100644 --- a/tests/core/modules/users/types.py +++ b/tests/core/modules/users/types.py @@ -31,10 +31,10 @@ class MyCustomString(str, fields.CustomStringField): class User(ResourceModel): - first_name: str - last_name: str + first_name: fields.TextWithKeyword + last_name: fields.TextWithKeyword email: Annotated[ - Optional[str], + Optional[fields.TextWithKeyword], validators.validate_email(), validators.validate_iunique_value_async(resource_name="users_async", field_name="email"), ] = None diff --git a/tests/core/resource_model_test.py b/tests/core/resource_model_test.py index 499d7fb5c4..0ad0f8ef41 100644 --- a/tests/core/resource_model_test.py +++ b/tests/core/resource_model_test.py @@ -62,9 +62,9 @@ def test_elastic_mapping(self): "_created": {"type": "date"}, "_updated": {"type": "date"}, "_etag": {"type": "text"}, - "first_name": {"type": "text"}, - "last_name": {"type": "text"}, - "email": {"type": "text"}, + "first_name": {"type": "text", "fields": {"keyword": {"type": "keyword"}}}, + "last_name": {"type": "text", "fields": {"keyword": {"type": "keyword"}}}, + "email": {"type": "text", "fields": {"keyword": {"type": "keyword"}}}, "name": {"type": "text", "fields": {"keyword": {"type": "keyword"}}}, "username": {"type": "text"}, "code": {"type": "keyword"}, diff --git a/tests/core/resource_service_test.py b/tests/core/resource_service_test.py index d7ccf1068e..9f0903192f 100644 --- a/tests/core/resource_service_test.py +++ b/tests/core/resource_service_test.py @@ -5,11 +5,13 @@ from bson import ObjectId from superdesk.core.types import SearchRequest +from superdesk.core.elastic.base_client import ElasticCursor from superdesk.utc import utcnow from superdesk.utils import format_time from superdesk.tests import AsyncTestCase + from .modules.users import UserResourceService from .fixtures.users import all_users, john_doe @@ -338,3 +340,73 @@ async def test_elastic_find(self): req = SearchRequest(args={"source": json.dumps(find_query)}) cursor = await self.service.find(req) self.assertEqual(await cursor.count(), 2) + + async def test_sort_param(self): + users = all_users() + await self.service.create(users) + + items = await (await self.service.find({})).to_list() + self.assertEqual(len(items), 3) + self.assertEqual(items[0].id, users[0].id) + self.assertEqual(items[1].id, users[1].id) + self.assertEqual(items[2].id, users[2].id) + + items = await ( + await self.service.find({}, sort=[("last_name.keyword", 1), ("first_name.keyword", 1)]) + ).to_list_raw() + self.assertEqual(len(items), 3) + self.assertDictContains(items[0], dict(first_name="Foo", last_name="Bar")) + self.assertDictContains(items[1], dict(first_name="Jane", last_name="Doe")) + self.assertDictContains(items[2], dict(first_name="John", last_name="Doe")) + + items = await ( + await self.service.find({}, sort=[("last_name.keyword", -1), ("first_name.keyword", -1)]) + ).to_list_raw() + self.assertEqual(len(items), 3) + self.assertDictContains(items[0], dict(first_name="John", last_name="Doe")) + self.assertDictContains(items[1], dict(first_name="Jane", last_name="Doe")) + self.assertDictContains(items[2], dict(first_name="Foo", last_name="Bar")) + + async def test_find_overloads(self): + await self.service.create(all_users()) + + async def assert_es_find_called_with(*args, **kwargs): + expected = kwargs.pop("expected") + self.service.elastic.find = mock.AsyncMock(return_value=(ElasticCursor(), 0)) + await self.service.find(*args, **kwargs) + self.service.elastic.find.assert_called_once_with(expected) + + # Test without any arguments + await assert_es_find_called_with( + SearchRequest(), expected=SearchRequest(where=None, page=1, max_results=25, sort=None) + ) + expected = SearchRequest() + await assert_es_find_called_with(SearchRequest(), expected=expected) + await assert_es_find_called_with({}, expected=expected) + + sort_query = [("last_name.keyword", 1), ("first_name.keyword", 1)] + expected = SearchRequest(sort=sort_query) + await assert_es_find_called_with(SearchRequest(sort=sort_query), expected=expected) + await assert_es_find_called_with({}, sort=sort_query, expected=expected) + + kwargs = dict( + page=2, + max_results=5, + sort=sort_query, + ) + expected = SearchRequest(**kwargs) + await assert_es_find_called_with(SearchRequest(**kwargs), expected=expected) + await assert_es_find_called_with({}, **kwargs, expected=expected) + + # Test with default sort in the resource config + sort_query = [("email.keyword", 1)] + self.service.config.default_sort = sort_query + expected = SearchRequest(sort=sort_query) + await assert_es_find_called_with(SearchRequest(), expected=expected) + await assert_es_find_called_with({}, expected=expected) + + # Test passing in sort param with default sort configured + custom_sort_query = [("scores", 1)] + expected = SearchRequest(sort=custom_sort_query) + await assert_es_find_called_with(SearchRequest(sort=custom_sort_query), expected=expected) + await assert_es_find_called_with({}, sort=custom_sort_query, expected=expected) From 76ee124dcc5943b842d3d665ce21ad993c4c830b Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 4 Sep 2024 15:26:44 +1000 Subject: [PATCH 07/17] update docs --- docs/core/resource_management.rst | 10 ---------- docs/core/types.rst | 20 ++++++++++++++++++++ docs/index.rst | 1 + 3 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 docs/core/types.rst diff --git a/docs/core/resource_management.rst b/docs/core/resource_management.rst index 466e9a6c5b..ec4ec445a9 100644 --- a/docs/core/resource_management.rst +++ b/docs/core/resource_management.rst @@ -62,14 +62,6 @@ Search References :member-order: bysource :members: -.. autoclass:: superdesk.core.resources.cursor.SearchArgs - :member-order: bysource - :members: - -.. autoclass:: superdesk.core.resources.cursor.SearchRequest - :member-order: bysource - :members: - .. autoclass:: superdesk.core.resources.cursor.ResourceCursorAsync :member-order: bysource :members: @@ -81,5 +73,3 @@ Search References .. autoclass:: superdesk.core.resources.cursor.MongoResourceCursorAsync :member-order: bysource :members: - -.. autodata:: superdesk.core.resources.cursor.ProjectedFieldArg diff --git a/docs/core/types.rst b/docs/core/types.rst new file mode 100644 index 0000000000..b5312d676c --- /dev/null +++ b/docs/core/types.rst @@ -0,0 +1,20 @@ +.. core_types: + +Types +===== + +.. autodata:: superdesk.core.types.ProjectedFieldArg + +.. autodata:: superdesk.core.types.SortListParam + +.. autodata:: superdesk.core.types.SortParam + +.. autodata:: superdesk.core.types.VersionParam + +.. autoclass:: superdesk.core.types.SearchArgs + :member-order: bysource + :members: + +.. autoclass:: superdesk.core.types.SearchRequest + :member-order: bysource + :members: diff --git a/docs/index.rst b/docs/index.rst index 913c8932a5..e3761e06f9 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -51,6 +51,7 @@ existing code to the new framework. core/mongo core/elastic core/storage + core/types .. _reference: From 9a309638afc913d00937d5492e27f483c12d1039 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 4 Sep 2024 15:43:47 +1000 Subject: [PATCH 08/17] fix mypy error --- superdesk/core/web/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superdesk/core/web/types.py b/superdesk/core/web/types.py index bcc5da5e46..edb7bbf200 100644 --- a/superdesk/core/web/types.py +++ b/superdesk/core/web/types.py @@ -137,7 +137,7 @@ def __init__( def __call__(self, args: Dict[str, Any], params: Dict[str, Any], request: "Request"): func_params = signature(self.func).parameters if not len(func_params): - return self.func() + return self.func() # type: ignore[call-arg,arg-type] elif "args" not in func_params and "params" not in func_params: return self.func(request) # type: ignore[call-arg,arg-type] From bf7053ec2646b56eeb08438d93ea45caa770dab0 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 14:23:48 +1000 Subject: [PATCH 09/17] support source attribute --- superdesk/core/elastic/resources.py | 7 ++++--- superdesk/core/mongo.py | 7 +++++-- superdesk/core/resources/model.py | 5 +++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/superdesk/core/elastic/resources.py b/superdesk/core/elastic/resources.py index f8e5229ab1..d944e1f7fe 100644 --- a/superdesk/core/elastic/resources.py +++ b/superdesk/core/elastic/resources.py @@ -88,12 +88,13 @@ def register_resource_config( client_config = ElasticClientConfig.create_from_dict( self.app.wsgi.config, prefix=resource_config.prefix or "ELASTICSEARCH", freeze=False ) - client_config.index += f"_{resource_name}" + source_name = self.app.resources.get_config(resource_name).datasource_name or resource_name + client_config.index += f"_{source_name}" client_config.set_frozen(True) - self._resource_clients[resource_name] = ElasticResourceClient(resource_name, client_config, resource_config) + self._resource_clients[resource_name] = ElasticResourceClient(source_name, client_config, resource_config) self._resource_async_clients[resource_name] = ElasticResourceAsyncClient( - resource_name, client_config, resource_config + source_name, client_config, resource_config ) def get_client(self, resource_name) -> ElasticResourceClient: diff --git a/superdesk/core/mongo.py b/superdesk/core/mongo.py index e9a27a6fe5..f0404f8af0 100644 --- a/superdesk/core/mongo.py +++ b/superdesk/core/mongo.py @@ -279,7 +279,8 @@ def get_collection(self, resource_name) -> Collection: :raises KeyError: if a resource with the provided ``resource_name`` is not registered """ - return self.get_db(resource_name).get_collection(resource_name) + source_name = self.app.resources.get_config(resource_name).datasource_name or resource_name + return self.get_db(resource_name).get_collection(source_name) def create_resource_indexes(self, resource_name: str, ignore_duplicate_keys=False): """Creates indexes for a resource @@ -376,7 +377,9 @@ def get_collection_async(self, resource_name: str) -> AsyncIOMotorCollection: :raises KeyError: if a resource with the provided ``resource_name`` is not registered """ - return self.get_db_async(resource_name).get_collection(resource_name) + resource_config = self.app.resources.get_config(resource_name) + source_name = resource_config.datasource_name or resource_config.name + return self.get_db_async(resource_name).get_collection(source_name) from .app import SuperdeskAsyncApp # noqa: E402 diff --git a/superdesk/core/resources/model.py b/superdesk/core/resources/model.py index a6580b3fb7..5feb08e615 100644 --- a/superdesk/core/resources/model.py +++ b/superdesk/core/resources/model.py @@ -205,6 +205,9 @@ class ResourceConfig: #: Optional sorting for this resource default_sort: SortListParam | None = None + #: Optionally override the name used for the MongoDB/Elastic sources + datasource_name: str | None = None + class Resources: """A high level resource class used to manage all resources in the system""" @@ -236,6 +239,8 @@ def register(self, config: ResourceConfig): self._resource_configs[config.name] = config config.data_class.model_resource_name = config.name + if not config.datasource_name: + config.datasource_name = config.name self.app.mongo.register_resource_config( config.name, From d37e1d0fd9d651666a664b4948914d318adb1d42 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 14:24:01 +1000 Subject: [PATCH 10/17] support parent links in REST endpoints --- superdesk/core/resources/__init__.py | 4 +- .../core/resources/resource_rest_endpoints.py | 163 +++++++++++++++++- superdesk/core/resources/service.py | 24 ++- superdesk/core/web/rest_endpoints.py | 13 +- superdesk/core/web/types.py | 6 + superdesk/factory/app.py | 6 + 6 files changed, 200 insertions(+), 16 deletions(-) diff --git a/superdesk/core/resources/__init__.py b/superdesk/core/resources/__init__.py index d9a05f451c..59f0553736 100644 --- a/superdesk/core/resources/__init__.py +++ b/superdesk/core/resources/__init__.py @@ -9,7 +9,7 @@ # at https://www.sourcefabric.org/superdesk/license from .model import Resources, ResourceModel, ResourceModelWithObjectId, ResourceConfig, dataclass -from .resource_rest_endpoints import RestEndpointConfig +from .resource_rest_endpoints import RestEndpointConfig, RestParentLink, get_id_url_type from .service import AsyncResourceService from ..mongo import MongoResourceConfig, MongoIndexOptions from ..elastic.resources import ElasticResourceConfig @@ -22,6 +22,8 @@ "dataclass", "fields", "RestEndpointConfig", + "RestParentLink", + "get_id_url_type", "AsyncResourceService", "MongoResourceConfig", "MongoIndexOptions", diff --git a/superdesk/core/resources/resource_rest_endpoints.py b/superdesk/core/resources/resource_rest_endpoints.py index ad1b94044f..4950fd7f17 100644 --- a/superdesk/core/resources/resource_rest_endpoints.py +++ b/superdesk/core/resources/resource_rest_endpoints.py @@ -8,14 +8,16 @@ # AUTHORS and LICENSE files distributed with this source code, or # at https://www.sourcefabric.org/superdesk/license -from typing import List, Optional, cast, Dict, Any, TypedDict, Type +from typing import List, Optional, cast, Dict, Any, Type import math from dataclasses import dataclass from pydantic import ValidationError from eve.utils import querydef from werkzeug.datastructures import MultiDict +from bson import ObjectId +from superdesk.core import json from superdesk.core.app import get_current_async_app from superdesk.errors import SuperdeskApiError from superdesk.core.types import SearchRequest, SearchArgs @@ -28,6 +30,31 @@ from .utils import resource_uses_objectid_for_id +@dataclass +class RestParentLink: + #: Name of the resource this parent link belongs to + resource_name: str + + #: Field used to store the resource ID in the child resource, defaults to ``resource_name`` + model_id_field: str | None = None + + #: Name of the URL argument in the route, defaults to ``model_id_field`` + url_arg_name: str | None = None + + #: ID Field of the parent used when searching for parent item resource, defaults to ``model_id_field`` + parent_id_field: str = "_id" + + def get_model_id_field(self) -> str: + """Get the ID Field for the local model used to store the reference to the parent model""" + + return self.model_id_field or self.resource_name + + def get_url_arg_name(self) -> str: + """Get the name of hte URL argument used in the route""" + + return self.url_arg_name or self.model_id_field or self.resource_name + + @dataclass class RestEndpointConfig: #: Optional list of resource level methods, defaults to ["GET", "POST"] @@ -42,6 +69,13 @@ class RestEndpointConfig: #: Optionally set a custom URL ID param syntax for item routes id_param_type: Optional[str] = None + #: Optionally set a custom URL for routes, defaults to ``resource_name`` + url: str | None = None + + #: Optionally assign parent resource(s) for this resource (parent/child relationship) + #: This will prepend this resources URL with the URL of the parent resource item + parent_links: list[RestParentLink] | None = None + def get_id_url_type(data_class: type[ResourceModel]) -> str: """Get the URL param type for the ID field for route registration""" @@ -66,16 +100,102 @@ def __init__( resource_config: ResourceConfig, endpoint_config: RestEndpointConfig, ): + self.resource_config = resource_config + self.endpoint_config = endpoint_config super().__init__( - url=resource_config.name, + url=endpoint_config.url or resource_config.name, name=resource_config.name, import_name=resource_config.__module__, resource_methods=endpoint_config.resource_methods, item_methods=endpoint_config.item_methods, id_param_type=endpoint_config.id_param_type or get_id_url_type(resource_config.data_class), ) - self.resource_config = resource_config - self.endpoint_config = endpoint_config + + def get_resource_url(self) -> str: + """Returns the URL for this resource + + If the resource has ``parent_links`` configured, these will be used to construct the URL + with the parent resources URL and item ID + """ + + if self.endpoint_config.parent_links is None: + return self.url + + app = get_current_async_app() + url = "" + for parent_link in self.endpoint_config.parent_links: + parent_config = app.resources.get_config(parent_link.resource_name) + id_param_type = 'regex("[\w,.:_-]+")' + parent_url = parent_link.resource_name + + if parent_config.rest_endpoints is not None: + if parent_config.rest_endpoints.url: + parent_url = parent_config.rest_endpoints.url + + if parent_config.rest_endpoints.id_param_type: + id_param_type = parent_config.rest_endpoints.id_param_type + else: + id_param_type = get_id_url_type(parent_config.data_class) + + arg_name = parent_link.get_url_arg_name() + url_prefix = f"{parent_url}/<{id_param_type}:{arg_name}>" + url += url_prefix + "/" + + return url + self.url + + def get_item_url(self, arg_name: str = "item_id") -> str: + """Returns the URL for an item of this resource + + :param arg_name: The name of the URL argument to use for the resource item URL + :return: The URL for an item of this resource + """ + + return f"{self.get_resource_url()}/<{self.id_param_type}:{arg_name}>" + + async def get_parent_items(self, request: Request) -> dict[str, dict]: + """Returns a dictionary of resource name to item for configured parent links + + :return: A dictionary, with the key being the resource name and value being the parent item + :raises SuperdeskApiError.badRequestError: If a parent item is not found + """ + + if self.endpoint_config.parent_links is None: + return {} + + items: dict[str, dict] = {} + for parent_link in self.endpoint_config.parent_links: + service = get_current_async_app().resources.get_resource_service(parent_link.resource_name) + item_id = request.get_view_args(parent_link.get_url_arg_name()) + if not item_id: + raise SuperdeskApiError.badRequestError("Parent resource ID not provided in URL") + item = await service.find_one_raw(use_mongo=True, **{parent_link.parent_id_field: item_id}) + if not item: + raise SuperdeskApiError.notFoundError( + f"Parent resource {parent_link.resource_name} with ID '{item_id}' not found" + ) + items[parent_link.resource_name] = item + + return items + + def construct_parent_item_lookup(self, request: Request) -> dict: + """Prefills a MongoDB query with the parent attributes from the request + + This is used to filter items of this resource to make sure they belong to all parent item(s). + + :param request: The request object currently being processed + :return: A MongoDB query + """ + if self.endpoint_config.parent_links is None: + return {} + + lookup = {} + for parent_link in self.endpoint_config.parent_links: + service = get_current_async_app().resources.get_resource_service(parent_link.resource_name) + item_id: str | ObjectId | None = request.get_view_args(parent_link.get_url_arg_name()) + if service.id_uses_objectid(): + item_id = ObjectId(item_id) + lookup[parent_link.get_model_id_field()] = item_id + return lookup async def process_get_item_request( self, @@ -85,8 +205,17 @@ async def process_get_item_request( ) -> Response: """Processes a get single item request""" + await self.get_parent_items(request) + service = get_current_async_app().resources.get_resource_service(self.resource_config.name) - item = await service.find_by_id_raw(args.item_id) + + if self.endpoint_config.parent_links: + lookup = self.construct_parent_item_lookup(request) + lookup["_id"] = args.item_id if not service.id_uses_objectid() else ObjectId(args.item_id) + item = await service.find_one_raw(use_mongo=True, **lookup) + else: + item = await service.find_by_id_raw(args.item_id) + if not item: raise SuperdeskApiError.notFoundError( f"{self.resource_config.name} resource with ID '{args.item_id}' not found" @@ -101,6 +230,8 @@ async def process_get_item_request( async def process_post_item_request(self, request: Request) -> Response: """Processes a create item request""" + parent_items = await self.get_parent_items(request) + service = get_current_async_app().resources.get_resource_service(self.resource_config.name) payload = await request.get_json() @@ -116,6 +247,12 @@ async def process_post_item_request(self, request: Request) -> Response: try: if "_id" not in value: value["_id"] = service.generate_id() + + for parent_link in self.endpoint_config.parent_links or []: + parent_item = parent_items.get(parent_link.resource_name) + if parent_item is not None: + value[parent_link.get_model_id_field()] = parent_item[parent_link.parent_id_field] + model_instance = self.resource_config.data_class.model_validate(value) model_instances.append(model_instance) except ValidationError as validation_error: @@ -132,6 +269,8 @@ async def process_patch_item_request( ) -> Response: """Processes an update item request""" + await self.get_parent_items(request) + service = get_current_async_app().resources.get_resource_service(self.resource_config.name) payload = await request.get_json() @@ -154,6 +293,8 @@ async def process_patch_item_request( async def process_delete_item_request(self, args: ItemRequestViewArgs, params: None, request: Request) -> Response: """Processes a delete item request""" + await self.get_parent_items(request) + service = get_current_async_app().resources.get_resource_service(self.resource_config.name) original = await service.find_by_id(args.item_id) @@ -179,6 +320,18 @@ async def process_get_request( ) -> Response: """Processes a search request""" + await self.get_parent_items(request) + + if len(self.endpoint_config.parent_links or []): + if not isinstance(params.where, dict): + if params.where is None: + params.where = {} + elif isinstance(params.where, str): + params.where = cast(dict, json.loads(params.where)) + + lookup = self.construct_parent_item_lookup(request) + params.where.update(lookup) + service = get_current_async_app().resources.get_resource_service(self.resource_config.name) params.args = cast(SearchArgs, params.model_extra) cursor = await service.find(params) diff --git a/superdesk/core/resources/service.py b/superdesk/core/resources/service.py index dbd1947e26..d8aac11624 100644 --- a/superdesk/core/resources/service.py +++ b/superdesk/core/resources/service.py @@ -36,7 +36,7 @@ from superdesk.errors import SuperdeskApiError from superdesk.utc import utcnow from superdesk.json_utils import SuperdeskJSONEncoder -from superdesk.core.types import SearchRequest, SortListParam, SortParam +from superdesk.core.types import SearchRequest, SortParam from ..app import SuperdeskAsyncApp, get_current_async_app from .fields import ObjectId as ObjectIdField @@ -112,22 +112,32 @@ def get_model_instance_from_dict(self, data: Dict[str, Any]) -> ResourceModelTyp data.pop("_type", None) return cast(ResourceModelType, self.config.data_class.model_validate(data)) - async def find_one(self, **lookup) -> Optional[ResourceModelType]: + async def find_one_raw(self, use_mongo: bool = False, **lookup) -> dict | None: """Find a resource by ID + :param use_mongo: If ``True`` will force use mongo, else will attempt elastic first :param lookup: Dictionary of key/value pairs used to find the document :return: ``None`` if resource not found, otherwise an instance of ``ResourceModel`` for this resource """ try: - item = await self.elastic.find_one(**lookup) + if not use_mongo: + return await self.elastic.find_one(**lookup) except KeyError: - item = await self.mongo.find_one(lookup) + pass - if item is None: - return None + return await self.mongo.find_one(lookup) + + async def find_one(self, use_mongo: bool = False, **lookup) -> Optional[ResourceModelType]: + """Find a resource by ID + + :param use_mongo: If ``True`` will force use mongo, else will attempt elastic first + :param lookup: Dictionary of key/value pairs used to find the document + :return: ``None`` if resource not found, otherwise an instance of ``ResourceModel`` for this resource + """ - return self.get_model_instance_from_dict(item) + item = await self.find_one_raw(use_mongo=use_mongo, **lookup) + return None if not item else self.get_model_instance_from_dict(item) async def find_by_id(self, item_id: Union[str, ObjectId]) -> Optional[ResourceModelType]: """Find a resource by ID diff --git a/superdesk/core/web/rest_endpoints.py b/superdesk/core/web/rest_endpoints.py index 6bdb4e306e..edc9161fd7 100644 --- a/superdesk/core/web/rest_endpoints.py +++ b/superdesk/core/web/rest_endpoints.py @@ -50,10 +50,11 @@ def __init__( self.item_methods = item_methods or ["GET", "PATCH", "DELETE"] self.id_param_type = id_param_type or "string" + resource_url = self.get_resource_url() if "GET" in self.resource_methods: self.endpoints.append( Endpoint( - url=self.url, + url=resource_url, name="resource_get", func=self.process_get_request, methods=["GET"], @@ -63,14 +64,14 @@ def __init__( if "POST" in self.resource_methods: self.endpoints.append( Endpoint( - url=self.url, + url=resource_url, name="resource_post", func=self.process_post_item_request, methods=["POST"], ) ) - item_url = f"{self.url}/<{self.id_param_type}:item_id>" + item_url = self.get_item_url() if "GET" in self.item_methods: self.endpoints.append( Endpoint( @@ -101,6 +102,12 @@ def __init__( ) ) + def get_resource_url(self): + return self.url + + def get_item_url(self, arg_name: str = "item_id"): + return f"{self.get_resource_url()}/<{self.id_param_type}:{arg_name}>" + async def process_get_item_request( self, args: ItemRequestViewArgs, diff --git a/superdesk/core/web/types.py b/superdesk/core/web/types.py index edb7bbf200..0d22c4d18f 100644 --- a/superdesk/core/web/types.py +++ b/superdesk/core/web/types.py @@ -196,6 +196,12 @@ async def get_data(self) -> Union[bytes, str]: async def abort(self, code: int, *args: Any, **kwargs: Any) -> NoReturn: ... + def get_view_args(self, key: str) -> str | None: + ... + + def get_url_arg(self, key: str) -> str | None: + ... + class EndpointGroup: """Base class used for registering a group of endpoints""" diff --git a/superdesk/factory/app.py b/superdesk/factory/app.py index 98ea23a972..991150204a 100644 --- a/superdesk/factory/app.py +++ b/superdesk/factory/app.py @@ -79,6 +79,12 @@ async def get_data(self) -> Union[bytes, str]: async def abort(self, code: int, *args: Any, **kwargs: Any) -> NoReturn: abort(code, *args, **kwargs) + def get_view_args(self, key: str) -> str | None: + return None if not self.request.view_args else self.request.view_args.get(key, None) + + def get_url_arg(self, key: str) -> str | None: + return self.request.args.get(key, None) + def set_error_handlers(app): """Set error handlers for the given application object. From 655cfacb33c88d1546a9c6838f56c2853901f288 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 14:24:12 +1000 Subject: [PATCH 11/17] improve core test performance --- superdesk/tests/__init__.py | 50 ++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/superdesk/tests/__init__.py b/superdesk/tests/__init__.py index da9b2b8435..14da8aa7cf 100644 --- a/superdesk/tests/__init__.py +++ b/superdesk/tests/__init__.py @@ -67,7 +67,7 @@ def get_mongo_uri(key, dbname): return "/".join([env_host, dbname]) -def update_config(conf): +def update_config(conf, auto_add_apps: bool = True): conf["ELASTICSEARCH_INDEX"] = "sptest" conf["MONGO_DBNAME"] = "sptests" conf["MONGO_URI"] = get_mongo_uri("MONGO_URI", "sptests") @@ -100,7 +100,8 @@ def update_config(conf): conf["MACROS_MODULE"] = "superdesk.macros" conf["DEFAULT_TIMEZONE"] = "Europe/Prague" conf["LEGAL_ARCHIVE"] = True - conf["INSTALLED_APPS"].extend(["planning", "superdesk.macros.imperial", "apps.rundowns"]) + if auto_add_apps: + conf["INSTALLED_APPS"].extend(["planning", "superdesk.macros.imperial", "apps.rundowns"]) # limit mongodb connections conf["MONGO_CONNECT"] = False @@ -176,7 +177,7 @@ async def drop_mongo(app): dbconn.drop_database(dbname) -def setup_config(config): +def setup_config(config, auto_add_apps: bool = True): app_abspath = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) app_config = Config(app_abspath) app_config.from_object("superdesk.default_settings") @@ -190,7 +191,7 @@ def setup_config(config): else: logger.warning("Can't find local settings") - update_config(app_config) + update_config(app_config, auto_add_apps) app_config.setdefault("INSTALLED_APPS", []) @@ -360,8 +361,8 @@ def inner(*a, **kw): use_snapshot.cache = {} # type: ignore -async def setup(context=None, config=None, app_factory=get_app, reset=False): - if not hasattr(setup, "app") or setup.reset or config: +async def setup(context=None, config=None, app_factory=get_app, reset=False, auto_add_apps: bool = True): + if not hasattr(setup, "app") or setup.reset or config: # type: ignore[attr-defined] if hasattr(setup, "app"): # Close all PyMongo Connections (new ones will be created with ``app_factory`` call) for key, val in setup.app.extensions["pymongo"].items(): @@ -370,10 +371,10 @@ async def setup(context=None, config=None, app_factory=get_app, reset=False): if getattr(setup.app, "async_app", None): setup.app.async_app.stop() - cfg = setup_config(config) - setup.app = app_factory(cfg) - setup.reset = reset - app = setup.app + cfg = setup_config(config, auto_add_apps) + setup.app = app_factory(cfg) # type: ignore[attr-defined] + setup.reset = reset # type: ignore[attr-defined] + app = setup.app # type: ignore[attr-defined] if context: context.app = app @@ -542,6 +543,9 @@ def setupApp(self): self.app_config = setup_config(self.app_config) self.app = SuperdeskAsyncApp(MockWSGI(config=self.app_config)) + self.startApp() + + def startApp(self): self.app.start() async def asyncSetUp(self): @@ -574,8 +578,19 @@ def model_instance_to_json(self, model_instance: ResourceModel): return model_instance.model_dump(by_alias=True, exclude_unset=True, mode="json") async def post(self, *args, **kwargs) -> Response: - if "json" in kwargs and isinstance(kwargs["json"], ResourceModel): - kwargs["json"] = self.model_instance_to_json(kwargs["json"]) + if "json" in kwargs: + if isinstance(kwargs["json"], ResourceModel): + kwargs["json"] = self.model_instance_to_json(kwargs["json"]) + elif isinstance(kwargs["json"], list): + kwargs["json"] = [ + self.model_instance_to_json(item) if isinstance(item, ResourceModel) else item + for item in kwargs["json"] + ] + elif isinstance(kwargs["json"], dict): + kwargs["json"] = { + key: self.model_instance_to_json(value) if isinstance(value, ResourceModel) else value + for key, value in kwargs["json"].items() + } return await super().post(*args, **kwargs) @@ -583,13 +598,19 @@ async def post(self, *args, **kwargs) -> Response: class AsyncFlaskTestCase(AsyncTestCase): async_app: SuperdeskAsyncApp app: SuperdeskApp + use_default_apps: bool = False async def asyncSetUp(self): if getattr(self, "async_app", None): self.async_app.stop() await self.async_app.elastic.stop() - await setup(self, config=self.app_config, reset=True) + if self.use_default_apps: + await setup(self, config=self.app_config, reset=True, auto_add_apps=True) + else: + self.app_config.setdefault("CORE_APPS", []) + self.app_config.setdefault("INSTALLED_APPS", []) + await setup(self, config=self.app_config, reset=True, auto_add_apps=False) self.async_app = self.app.async_app self.app.test_client_class = TestClient self.test_client = self.app.test_client() @@ -618,4 +639,5 @@ async def get_resource_etag(self, resource: str, item_id: str): return (await (await self.test_client.get(f"/api/{resource}/{item_id}")).get_json())["_etag"] -TestCase = AsyncFlaskTestCase +class TestCase(AsyncFlaskTestCase): + use_default_apps: bool = True From acc663cabfa75c8a41d7869f5585b3312c920423 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 14:24:22 +1000 Subject: [PATCH 12/17] add tests --- tests/core/modules/company.py | 20 ++++ tests/core/modules/topics.py | 74 +++++++++++++++ tests/core/mongo_test.py | 23 +++++ tests/core/resource_parent_links.py | 138 ++++++++++++++++++++++++++++ 4 files changed, 255 insertions(+) create mode 100644 tests/core/modules/company.py create mode 100644 tests/core/modules/topics.py create mode 100644 tests/core/resource_parent_links.py diff --git a/tests/core/modules/company.py b/tests/core/modules/company.py new file mode 100644 index 0000000000..211f86fbae --- /dev/null +++ b/tests/core/modules/company.py @@ -0,0 +1,20 @@ +from superdesk.core.module import Module +from superdesk.core.resources import ResourceConfig, ResourceModel, AsyncResourceService, RestEndpointConfig + + +class CompanyResource(ResourceModel): + name: str + + +class CompanyService(AsyncResourceService[CompanyResource]): + resource_name = "companies" + + +companies_resource_config = ResourceConfig( + name="companies", + data_class=CompanyResource, + service=CompanyService, + rest_endpoints=RestEndpointConfig(), +) + +module = Module(name="tests.company", resources=[companies_resource_config]) diff --git a/tests/core/modules/topics.py b/tests/core/modules/topics.py new file mode 100644 index 0000000000..04722323ae --- /dev/null +++ b/tests/core/modules/topics.py @@ -0,0 +1,74 @@ +from typing import Annotated +from superdesk.core.module import Module +from superdesk.core.resources import ( + ResourceConfig, + ResourceModel, + AsyncResourceService, + RestEndpointConfig, + RestParentLink, +) +from superdesk.core.resources.validators import validate_data_relation_async + +from .users import user_model_config +from .company import companies_resource_config + + +class TopicFolder(ResourceModel): + name: str + section: str + + +class UserFolder(TopicFolder): + user: Annotated[str, validate_data_relation_async(user_model_config.name)] + + +class UserFolderService(AsyncResourceService[UserFolder]): + resource_name = "user_topic_folders" + + +user_folder_config = ResourceConfig( + name="user_topic_folders", + datasource_name="topic_folders", + data_class=UserFolder, + service=UserFolderService, + rest_endpoints=RestEndpointConfig( + parent_links=[ + RestParentLink( + resource_name=user_model_config.name, + model_id_field="user", + ) + ], + url="topic_folders", + ), +) + + +class CompanyFolder(TopicFolder): + company: str + + +class CompanyFolderService(AsyncResourceService[CompanyFolder]): + resource_name = "company_topic_folders" + + +company_folder_config = ResourceConfig( + name="company_topic_folders", + datasource_name="topic_folders", + data_class=CompanyFolder, + service=CompanyFolderService, + rest_endpoints=RestEndpointConfig( + parent_links=[ + RestParentLink( + resource_name=companies_resource_config.name, + model_id_field="company", + ) + ], + url="topic_folders", + ), +) + + +module = Module( + name="tests.multi_sources", + resources=[user_folder_config, company_folder_config], +) diff --git a/tests/core/mongo_test.py b/tests/core/mongo_test.py index 10f1c5ef8c..c34251cd3b 100644 --- a/tests/core/mongo_test.py +++ b/tests/core/mongo_test.py @@ -85,3 +85,26 @@ def test_init_indexes(self): # ``collation`` uses an ``bson.son.SON` instance, so use that for testing here self.assertEqual(indexes["combined_name_1"]["collation"].get("locale"), "en") self.assertEqual(indexes["combined_name_1"]["collation"].get("strength"), 1) + + +class MongoClientSourceTestCase(AsyncTestCase): + app_config = { + "MODULES": [ + "tests.core.modules.users", + "tests.core.modules.company", + "tests.core.modules.topics", + ] + } + + def test_mongo_collection_source(self): + user_db = self.app.mongo.get_db("user_topic_folders") + user_collection = user_db.get_collection("topic_folders") + self.assertEqual(self.app.mongo.get_collection("user_topic_folders"), user_collection) + self.assertEqual(user_collection.full_name, "sptests.topic_folders") + + company_db = self.app.mongo.get_db("company_topic_folders") + company_collection = company_db.get_collection("topic_folders") + self.assertEqual(self.app.mongo.get_collection("company_topic_folders"), company_collection) + self.assertEqual(company_collection.full_name, "sptests.topic_folders") + + self.assertEqual(user_collection, company_collection) diff --git a/tests/core/resource_parent_links.py b/tests/core/resource_parent_links.py new file mode 100644 index 0000000000..7735c603aa --- /dev/null +++ b/tests/core/resource_parent_links.py @@ -0,0 +1,138 @@ +from superdesk.core.resources.resource_rest_endpoints import ResourceRestEndpoints +from superdesk.tests import AsyncFlaskTestCase + +from .fixtures.users import john_doe, jane_doe + + +class ResourceParentLinksTestCase(AsyncFlaskTestCase): + app_config = { + "MODULES": [ + "tests.core.modules.users", + "tests.core.modules.company", + "tests.core.modules.topics", + ] + } + + async def test_parent_url_links(self): + user_collection = self.async_app.mongo.get_collection_async("user_topic_folders") + company_collection = self.async_app.mongo.get_collection_async("company_topic_folders") + test_user1 = john_doe() + test_user2 = jane_doe() + + # First add the 2 users we'll use for filtering/testing + response = await self.test_client.post("/api/users_async", json=[test_user1, test_user2]) + self.assertEqual(response.status_code, 201) + + # Make sure the folders resource is empty + self.assertEqual(await user_collection.count_documents({}), 0) + response = await self.test_client.get(f"/api/users_async/{test_user1.id}/topic_folders") + self.assertEqual(len((await response.get_json())["_items"]), 0) + + # Add a folder for each user + response = await self.test_client.post( + f"/api/users_async/{test_user1.id}/topic_folders", json=dict(name="Sports", section="wire") + ) + self.assertEqual(response.status_code, 201) + response = await self.test_client.post( + f"/api/users_async/{test_user2.id}/topic_folders", json=dict(name="Finance", section="agenda") + ) + self.assertEqual(response.status_code, 201) + + # Make sure all folders exist in the mongo collection + self.assertEqual(await user_collection.count_documents({}), 2) + # points to same collection as users folders, so it too should have 2 documents + self.assertEqual(await company_collection.count_documents({}), 2) + + # Test getting folders for User1 + response = await self.test_client.get(f"/api/users_async/{test_user1.id}/topic_folders") + self.assertEqual(response.status_code, 200) + data = await response.get_json() + self.assertEqual(len(data["_items"]), 1) + self.assertEqual(data["_meta"]["total"], 1) + self.assertDictContains(data["_items"][0], dict(user=test_user1.id, name="Sports", section="wire")) + + # Test getting folders for User2 + response = await self.test_client.get(f"/api/users_async/{test_user2.id}/topic_folders") + self.assertEqual(response.status_code, 200) + data = await response.get_json() + self.assertEqual(len(data["_items"]), 1) + self.assertEqual(data["_meta"]["total"], 1) + self.assertDictContains(data["_items"][0], dict(user=test_user2.id, name="Finance", section="agenda")) + + # Test searching folders for User1 + user1_folders_url = f"/api/users_async/{test_user1.id}/topic_folders" + response = await self.test_client.post(user1_folders_url, json=dict(name="Finance", section="agenda")) + self.assertEqual(response.status_code, 201) + + # Make sure there are 2 folders when not filtering + response = await self.test_client.get(user1_folders_url) + self.assertEqual(response.status_code, 200) + data = await response.get_json() + self.assertEqual(len(data["_items"]), 2) + + # Make sure there is only 1 folder when filtering + response = await self.test_client.get(user1_folders_url + '?where={"section":"wire"}') + self.assertEqual(response.status_code, 200) + data = await response.get_json() + self.assertEqual(len(data["_items"]), 1) + self.assertEqual(data["_meta"]["total"], 1) + self.assertDictContains(data["_items"][0], dict(user=test_user1.id, name="Sports", section="wire")) + + async def test_patch_and_delete(self): + # Create the user + test_user1 = john_doe() + response = await self.test_client.post("/api/users_async", json=test_user1) + self.assertEqual(response.status_code, 201) + + # Create the users folder + response = await self.test_client.post( + f"/api/users_async/{test_user1.id}/topic_folders", json=dict(name="Sports", section="wire") + ) + self.assertEqual(response.status_code, 201) + folder_id = (await response.get_json())[0] + + # Get the folder, so we can use it's etag + response = await self.test_client.get(f"/api/users_async/{test_user1.id}/topic_folders/{folder_id}") + folder = await response.get_json() + + # Update the users folder + response = await self.test_client.patch( + f"/api/users_async/{test_user1.id}/topic_folders/{folder_id}", + json=dict(name="Swimming"), + headers={"If-Match": folder["_etag"]}, + ) + self.assertEqual(response.status_code, 200) + + # Delete the users folder + response = await self.test_client.get(f"/api/users_async/{test_user1.id}/topic_folders/{folder_id}") + folder = await response.get_json() + response = await self.test_client.delete( + f"/api/users_async/{test_user1.id}/topic_folders/{folder_id}", headers={"If-Match": folder["_etag"]} + ) + self.assertEqual(response.status_code, 204) + + async def test_parent_link_validation(self): + test_user1 = john_doe() + + # Test request returns 404 when parent item does not exist in the DB + response = await self.test_client.post( + f"/api/users_async/{test_user1.id}/topic_folders", json=dict(name="Sports", section="wire") + ) + self.assertEqual(response.status_code, 404) + + # Now add the parent item, and test request returns 201 + response = await self.test_client.post("/api/users_async", json=test_user1) + self.assertEqual(response.status_code, 201) + response = await self.test_client.post( + f"/api/users_async/{test_user1.id}/topic_folders", json=dict(name="Sports", section="wire") + ) + self.assertEqual(response.status_code, 201) + + def test_generated_resource_url(self): + config = self.async_app.resources.get_config("user_topic_folders") + endpoint = ResourceRestEndpoints(config, config.rest_endpoints) + self.assertEqual(endpoint.get_resource_url(), 'users_async//topic_folders') + + config = self.async_app.resources.get_config("company_topic_folders") + endpoint = ResourceRestEndpoints(config, config.rest_endpoints) + self.assertEqual(endpoint.get_resource_url(), 'companies//topic_folders') From 431d9ebd4da871249e6dd03ebd4e4d2182cabaa7 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 14:24:33 +1000 Subject: [PATCH 13/17] fix pytest-asyncio deprecation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 4d7f66b06d..7944e13c2f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,3 +21,4 @@ exclude = ''' testpaths = ["tests", "superdesk", "apps", "content_api"] python_files = "*_test.py *_tests.py test_*.py tests_*.py tests.py test.py" asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" From 976a71851c6dd770116f1e599c0a2ae3e1765370 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 14:24:42 +1000 Subject: [PATCH 14/17] update docs --- docs/core/web.rst | 138 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 1 deletion(-) diff --git a/docs/core/web.rst b/docs/core/web.rst index 4e2e7e58dd..e4fdff9ebf 100644 --- a/docs/core/web.rst +++ b/docs/core/web.rst @@ -135,7 +135,138 @@ For example:: rest_endpoints=RestEndpointConfig(), ) - module = Module(name="tests.users") + module = Module( + name="tests.users", + resources=[user_resource_config], + ) + + +Resource REST Endpoint with Parent +---------------------------------- + +REST endpoints can also include a parent/child relationship with the resource. This is achieved using the +:class:`RestParentLink ` +attribute on the RestEndpointConfig. + +Example config:: + + from typing import Annotated + from superdesk.core.module import Module + from superdesk.core.resources import ( + ResourceConfig, + ResourceModel, + RestEndpointConfig, + RestParentLink, + ) + from superdesk.core.resources.validators import ( + validate_data_relation_async, + ) + + # 1. Define parent resource and config + class Company(ResourceModel): + name: str + + company_resource_config = ResourceConfig( + name="companies", + data_class=Company, + rest_endpoints=RestEndpointConfig() + ) + + # 2. Define child resource and config + class User(ResourceModel): + first_name: str + last_name: str + + # 2a. Include a field that references the parent + company: Annotated[ + str, + validate_data_relation_async( + company_resource_config.name, + ), + ] + + user_resource_config = ResourceConfig( + name="users", + data_class=User, + rest_endpoints=RestEndpointConfig( + + # 2b. Include a link to Company as a parent resource + parent_links=[ + RestParentLink( + resource_name=company_resource_config.name, + model_id_field="company", + ), + ], + ), + ) + + # 3. Register the resources with a module + module = Module( + name="tests.users", + resources=[ + company_resource_config, + user_resource_config, + ], + ) + + +The above example exposes the following URLs: + +* /api/companies +* /api/companies/```` +* /api/companies/````/users +* /api/companies/````/users/```` + +As you can see the ``users`` endpoints are prefixed with ``/api/company//``. + +This provides the following functionality: + +* Validation that a Company must exist for the user +* Populates the ``company`` field of a User with the ID from the URL +* When searching for users, will only provide users for the specific company provided in the URL of the request + +For example:: + + async def test_users(): + # Create the parent Company + response = await client.post( + "/api/company", + json={"name": "Sourcefabric"} + ) + + # Retrieve the Company ID from the response + company_id = (await response.get_json())[0] + + # Attemps to create a user with non-existing company + # responds with a 404 - NotFound error + response = await client.post( + f"/api/company/blah_blah/users", + json={"first_name": "Monkey", "last_name": "Mania"} + ) + assert response.status_code == 404 + + # Create the new User + # Notice the ``company_id`` is used in the URL + response = await client.post( + f"/api/company/{company_id}/users", + json={"first_name": "Monkey", "last_name": "Mania"} + ) + user_id = (await response.get_json())[0] + + # Retrieve the new user + response = await client.get( + f"/api/company/{company_id}/users/{user_id}" + ) + user_dict = await response.get_json() + assert user_dict["company"] == company_id + + # Retrieve all company users + response = await client.get( + f"/api/company/{company_id}/users" + ) + users_dict = (await response.get_json())["_items"] + assert len(users_dict) == 1 + assert users_dict[0]["_id"] == user_id Validation @@ -240,6 +371,11 @@ API References :members: :undoc-members: +.. autoclass:: superdesk.core.resources.resource_rest_endpoints.RestParentLink + :member-order: bysource + :members: + :undoc-members: + .. autoclass:: superdesk.core.resources.resource_rest_endpoints.ResourceRestEndpoints :member-order: bysource :members: From 9511b8cd3707b64a44a7eb9301e0adce1ade3277 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 14:31:50 +1000 Subject: [PATCH 15/17] run black --- superdesk/core/resources/service.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/superdesk/core/resources/service.py b/superdesk/core/resources/service.py index 77be1bfbbe..7ac4bc755e 100644 --- a/superdesk/core/resources/service.py +++ b/superdesk/core/resources/service.py @@ -430,12 +430,14 @@ async def get_all_batch(self, size=500, max_iterations=10000, lookup=None) -> As logger.warning(f"Not enough iterations for resource {self.resource_name}") @overload - async def find(self, req: SearchRequest) -> ResourceCursorAsync[ResourceModelType]: ... + async def find(self, req: SearchRequest) -> ResourceCursorAsync[ResourceModelType]: + ... @overload async def find( self, req: dict, page: int = 1, max_results: int = 25, sort: SortParam | None = None - ) -> ResourceCursorAsync[ResourceModelType]: ... + ) -> ResourceCursorAsync[ResourceModelType]: + ... async def find( self, From 45645ccd08aa2784297353f8fc5e04f06b3b2591 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Wed, 11 Sep 2024 15:54:14 +1000 Subject: [PATCH 16/17] rename resource_parent_links file --- .../{resource_parent_links.py => resource_parent_links_test.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/core/{resource_parent_links.py => resource_parent_links_test.py} (100%) diff --git a/tests/core/resource_parent_links.py b/tests/core/resource_parent_links_test.py similarity index 100% rename from tests/core/resource_parent_links.py rename to tests/core/resource_parent_links_test.py From 12864ed956ea2949a8c5a04aa64acf20b3fd1303 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Mon, 16 Sep 2024 10:44:59 +1000 Subject: [PATCH 17/17] run black, pass version=None when getting parent item --- superdesk/core/resources/resource_rest_endpoints.py | 2 +- superdesk/core/resources/service.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/superdesk/core/resources/resource_rest_endpoints.py b/superdesk/core/resources/resource_rest_endpoints.py index edd59b2b28..65747e8035 100644 --- a/superdesk/core/resources/resource_rest_endpoints.py +++ b/superdesk/core/resources/resource_rest_endpoints.py @@ -175,7 +175,7 @@ async def get_parent_items(self, request: Request) -> dict[str, dict]: item_id = request.get_view_args(parent_link.get_url_arg_name()) if not item_id: raise SuperdeskApiError.badRequestError("Parent resource ID not provided in URL") - item = await service.find_one_raw(use_mongo=True, **{parent_link.parent_id_field: item_id}) + item = await service.find_one_raw(use_mongo=True, version=None, **{parent_link.parent_id_field: item_id}) if not item: raise SuperdeskApiError.notFoundError( f"Parent resource {parent_link.resource_name} with ID '{item_id}' not found" diff --git a/superdesk/core/resources/service.py b/superdesk/core/resources/service.py index 63dc967d33..bdf70b5f42 100644 --- a/superdesk/core/resources/service.py +++ b/superdesk/core/resources/service.py @@ -145,7 +145,9 @@ async def find_one_raw(self, use_mongo: bool = False, version: int | None = None return item - async def find_one(self, use_mongo: bool = False, version: int | None = None, **lookup) -> Optional[ResourceModelType]: + async def find_one( + self, use_mongo: bool = False, version: int | None = None, **lookup + ) -> Optional[ResourceModelType]: """Find a resource by ID :param use_mongo: If ``True`` will force use mongo, else will attempt elastic first