From e8274e915bc174f42e34de6b5bcc8109a66155e8 Mon Sep 17 00:00:00 2001 From: fderuiter <127706008+fderuiter@users.noreply.github.com> Date: Tue, 5 May 2026 18:14:44 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20Refactor:=20SDK=20Core?= =?UTF-8?q?=20-=20Centralize=20Endpoint=20Path=20Building=20and=20Not=20Fo?= =?UTF-8?q?und=20Logic=20(DRY)\n\n=E2=99=BB=EF=B8=8F=20DRY:=20Collapsed=20?= =?UTF-8?q?identical=20path=20building=20and=20404=20raising=20logic=20acr?= =?UTF-8?q?oss=20`PathGetEndpointMixin`,=20`ListEndpointMixin`,=20`FilterG?= =?UTF-8?q?etEndpointMixin`,=20`JobsEndpoint`,=20and=20`RecordsEndpoint`?= =?UTF-8?q?=20into=20`EndpointABC`.\n=F0=9F=A7=B1=20SOLID:=20Enforced=20In?= =?UTF-8?q?terface=20Segregation=20and=20Dependency=20Inversion=20by=20req?= =?UTF-8?q?uiring=20subclasses=20to=20implement=20base=20path=20properties?= =?UTF-8?q?=20while=20abstracting=20validation=20away=20from=20the=20mixin?= =?UTF-8?q?s.\n=F0=9F=93=89=20Diff:=20Removed=20duplicated=20generic=20pat?= =?UTF-8?q?h=20strings,=20redundant=20checks,=20and=20inline=20imports.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> --- src/imednet/core/endpoint/abc.py | 28 +++++++++++++++++++++ src/imednet/core/endpoint/mixins/get.py | 26 +++---------------- src/imednet/core/endpoint/mixins/list.py | 13 ++-------- src/imednet/core/endpoint/protocols.py | 8 ++++++ src/imednet/endpoints/jobs.py | 6 ----- src/imednet/endpoints/records.py | 4 +-- tests/unit/test_core_endpoint_mixins_get.py | 2 +- 7 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/imednet/core/endpoint/abc.py b/src/imednet/core/endpoint/abc.py index 5c2ffc76..912b98b7 100644 --- a/src/imednet/core/endpoint/abc.py +++ b/src/imednet/core/endpoint/abc.py @@ -71,3 +71,31 @@ def _validate_study_key(self, study_key: Optional[str]) -> None: from imednet.errors import ClientError raise ClientError("Study key must be provided or set in the context") + + def _get_endpoint_path(self, study_key: Optional[str], *extra_segments: Any) -> str: + """ + Build the endpoint path with optional study key and extra segments. + """ + self._validate_study_key(study_key) + segments = [] + if self.requires_study_key: + segments.append(study_key) + if self.PATH: + segments.append(self.PATH) + segments.extend(extra_segments) + return self._build_path(*segments) + + def _raise_not_found(self, study_key: Optional[str], item_id: Any = None) -> None: + """ + Raise a standardized NotFoundError. + """ + from imednet.errors import NotFoundError + + msg_parts = [f"{self.MODEL.__name__}"] + if item_id is not None: + msg_parts.append(str(item_id)) + msg_parts.append("not found") + if self.requires_study_key and study_key: + msg_parts.append(f"in study {study_key}") + + raise NotFoundError(" ".join(msg_parts)) diff --git a/src/imednet/core/endpoint/mixins/get.py b/src/imednet/core/endpoint/mixins/get.py index 5a9a2db7..6a5e0a44 100644 --- a/src/imednet/core/endpoint/mixins/get.py +++ b/src/imednet/core/endpoint/mixins/get.py @@ -1,13 +1,12 @@ from __future__ import annotations -from typing import Any, Iterable, List, Optional, cast +from typing import Any, List, Optional, cast from imednet.core.endpoint.abc import EndpointABC from imednet.core.endpoint.operations.filter_get import FilterGetOperation from imednet.core.endpoint.operations.get import PathGetOperation from imednet.core.paginator import AsyncPaginator, Paginator from imednet.core.protocols import AsyncRequestorProtocol, RequestorProtocol -from imednet.errors import NotFoundError from ..protocols import ListEndpointProtocol from .parsing import ParsingMixin, T @@ -22,12 +21,7 @@ class FilterGetEndpointMixin(EndpointABC[T]): def _validate_get_result(self, items: List[T], study_key: Optional[str], item_id: Any) -> T: if not items: - self._validate_study_key(study_key) - if self.requires_study_key: - raise NotFoundError( - f"{self.MODEL.__name__} {item_id} not found in study {study_key}" - ) - raise NotFoundError(f"{self.MODEL.__name__} {item_id} not found") + self._raise_not_found(study_key, item_id) return items[0] def _create_filter_operation( @@ -102,20 +96,6 @@ class PathGetEndpointMixin(ParsingMixin[T], EndpointABC[T]): # PATH is inherited from EndpointABC as abstract - def _get_path_for_id(self, study_key: Optional[str], item_id: Any) -> str: - segments: Iterable[Any] - self._validate_study_key(study_key) - if self.requires_study_key: - segments = (study_key, self.PATH, item_id) - else: - segments = (self.PATH, item_id) if self.PATH else (item_id,) - - # No cast needed as we inherit EndpointABC which defines _build_path - return self._build_path(*segments) - - def _raise_not_found(self, study_key: Optional[str], item_id: Any) -> None: - raise NotFoundError(f"{self.MODEL.__name__} not found") - def _create_path_operation(self, study_key: Optional[str], item_id: Any) -> PathGetOperation[T]: """ Create a PathGetOperation instance. @@ -127,7 +107,7 @@ def _create_path_operation(self, study_key: Optional[str], item_id: Any) -> Path Returns: An instantiated PathGetOperation. """ - path = self._get_path_for_id(study_key, item_id) + path = self._get_endpoint_path(study_key, item_id) return PathGetOperation[T]( path=path, parse_func=self._parse_item, diff --git a/src/imednet/core/endpoint/mixins/list.py b/src/imednet/core/endpoint/mixins/list.py index da89ece1..0f693de9 100644 --- a/src/imednet/core/endpoint/mixins/list.py +++ b/src/imednet/core/endpoint/mixins/list.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Any, Callable, Dict, Iterable, List, Optional, cast +from typing import Any, Callable, Dict, List, Optional, cast from imednet.constants import DEFAULT_PAGE_SIZE from imednet.core.endpoint.abc import EndpointABC @@ -22,15 +22,6 @@ class ListEndpointMixin(ParamMixin, CacheMixin[T], ParsingMixin[T], EndpointABC[ PAGINATOR_CLS: type[Paginator] = Paginator ASYNC_PAGINATOR_CLS: type[AsyncPaginator] = AsyncPaginator - def _get_path(self, study: Optional[str]) -> str: - segments: Iterable[Any] - self._validate_study_key(study) - if self.requires_study_key: - segments = (study, self.PATH) - else: - segments = (self.PATH,) if self.PATH else () - return self._build_path(*segments) - def _resolve_parse_func(self) -> Callable[[Any], T]: """ Resolve the parsing function to use for this endpoint. @@ -90,7 +81,7 @@ def _prepare_list_request( cached_result=cast(List[T], cached_result), ) - path = self._get_path(study) + path = self._get_endpoint_path(study) return ListRequestState( path=path, params=params, diff --git a/src/imednet/core/endpoint/protocols.py b/src/imednet/core/endpoint/protocols.py index 298bff70..102535bf 100644 --- a/src/imednet/core/endpoint/protocols.py +++ b/src/imednet/core/endpoint/protocols.py @@ -30,6 +30,14 @@ def _validate_study_key(self, study_key: Optional[str]) -> None: """Validate that a study key is provided if required.""" ... + def _get_endpoint_path(self, study_key: Optional[str], *extra_segments: Any) -> str: + """Build the API path with optional study key and extra segments.""" + ... + + def _raise_not_found(self, study_key: Optional[str], item_id: Any = None) -> None: + """Raise a standardized NotFoundError.""" + ... + @runtime_checkable class ListEndpointProtocol(Protocol[T]): diff --git a/src/imednet/endpoints/jobs.py b/src/imednet/endpoints/jobs.py index 72e55f23..81ddba67 100644 --- a/src/imednet/endpoints/jobs.py +++ b/src/imednet/endpoints/jobs.py @@ -1,12 +1,9 @@ """Endpoint for checking job status in a study.""" -from typing import Any, Optional - from imednet.core.endpoint.base import GenericEndpoint from imednet.core.endpoint.edc_mixin import EdcEndpointMixin from imednet.core.endpoint.mixins import ListEndpointMixin, PathGetEndpointMixin from imednet.core.paginator import AsyncJsonListPaginator, JsonListPaginator -from imednet.errors import NotFoundError from imednet.models.jobs import JobStatus @@ -27,6 +24,3 @@ class JobsEndpoint( MODEL = JobStatus PAGINATOR_CLS = JsonListPaginator ASYNC_PAGINATOR_CLS = AsyncJsonListPaginator - - def _raise_not_found(self, study_key: Optional[str], item_id: Any) -> None: - raise NotFoundError(f"Job {item_id} not found in study {study_key}") diff --git a/src/imednet/endpoints/records.py b/src/imednet/endpoints/records.py index 608832f7..cc3e8a55 100644 --- a/src/imednet/endpoints/records.py +++ b/src/imednet/endpoints/records.py @@ -51,7 +51,7 @@ def create( Raises: ClientError: If email_notify contains invalid characters """ - path = self._build_path(study_key, self.PATH) + path = self._get_endpoint_path(study_key) operation = RecordCreateOperation[Job]( path=path, records_data=records_data, @@ -90,7 +90,7 @@ async def async_create( Raises: ClientError: If email_notify contains invalid characters """ - path = self._build_path(study_key, self.PATH) + path = self._get_endpoint_path(study_key) operation = RecordCreateOperation[Job]( path=path, records_data=records_data, diff --git a/tests/unit/test_core_endpoint_mixins_get.py b/tests/unit/test_core_endpoint_mixins_get.py index a73fa967..b4376539 100644 --- a/tests/unit/test_core_endpoint_mixins_get.py +++ b/tests/unit/test_core_endpoint_mixins_get.py @@ -103,7 +103,7 @@ class DummyPathGetEndpointNoStudy(DummyPathGetEndpoint): def test_path_get_endpoint_no_study_key_no_path(): ep = DummyPathGetEndpointNoStudy() # It should generate path with just item_id - assert ep._get_path_for_id(None, 123) == "/123" + assert ep._get_endpoint_path(None, 123) == "/123" def test_path_get_endpoint_raise_not_found():