From 99d12304ad381308b7200b7a2c72c2e69fb6ebd4 Mon Sep 17 00:00:00 2001 From: sidkos Date: Sun, 5 Oct 2025 16:23:52 +0300 Subject: [PATCH 1/7] wip1 --- dp-client/dp_client/api/health.py | 14 ++ dp-client/dp_client/api/users.py | 119 ++++++++++-- dp-client/dp_client/client.py | 70 +++++++ src/api_spec_hash.md5 | 2 +- src/metadata_manager/serializers.py | 34 ++++ src/metadata_manager/urls.py | 14 +- src/metadata_manager/views.py | 17 +- src/open_api_spec.yml | 138 +++++++++++++- tests/component/test_user_model_component.py | 134 ++++++++++++- tests/conftest.py | 70 ++++++- tests/unit/test_user_model.py | 190 +++++++++++++------ 11 files changed, 715 insertions(+), 87 deletions(-) diff --git a/dp-client/dp_client/api/health.py b/dp-client/dp_client/api/health.py index 552683e..a7bc568 100644 --- a/dp-client/dp_client/api/health.py +++ b/dp-client/dp_client/api/health.py @@ -7,6 +7,15 @@ class HealthAPI: """Health endpoints wrapper using generated metadata_client api module.""" def __init__(self, client: Any) -> None: + """Initialize the HealthAPI wrapper. + + Args: + client: An instance of the generated metadata_client Client or + AuthenticatedClient used to perform HTTP requests. + + Raises: + RuntimeError: If the generated health endpoint cannot be imported. + """ try: from metadata_client.api.health import health_retrieve as _health_retrieve except Exception as exc: @@ -18,4 +27,9 @@ def __init__(self, client: Any) -> None: self._health_retrieve = _health_retrieve def health_check(self): + """Call GET /api/health/ and return the detailed response. + + Returns: + The detailed response object from the generated client. + """ return self._health_retrieve.sync_detailed(client=self._client) diff --git a/dp-client/dp_client/api/users.py b/dp-client/dp_client/api/users.py index 9f4b61e..d098135 100644 --- a/dp-client/dp_client/api/users.py +++ b/dp-client/dp_client/api/users.py @@ -1,34 +1,131 @@ from __future__ import annotations -from typing import Any, Union +from typing import Any, Dict, Union class UsersAPI: - """Users endpoints wrapper using generated metadata_client api module.""" + """Users endpoints wrapper using generated metadata_client api module. + + Implements a generalized approach to load and validate all user endpoints + at initialization time to avoid per-call checks and asserts. + """ + + # Mapping of logical names to attributes provided by metadata_client.api.users + _ENDPOINT_ATTRS: Dict[str, str] = { + "create": "users_create", + "list": "users_list", + "retrieve": "users_retrieve", + "update": "users_update", + "partial_update": "users_partial_update", + "destroy": "users_destroy", + } def __init__(self, client: Any) -> None: + """Initialize the UsersAPI wrapper. + + Args: + client: An instance of the generated metadata_client Client or + AuthenticatedClient that will be used to perform HTTP requests. + + Raises: + RuntimeError: If the metadata_client package or required modules + cannot be imported, or if any of the expected endpoints are + missing from metadata_client.api.users. + """ try: - from metadata_client.api.users import users_create as _users_create - from metadata_client.api.users import users_list as _users_list - from metadata_client.api.users import users_retrieve as _users_retrieve + from metadata_client.api import users as _users_api from metadata_client.models import User as _User except Exception as exc: raise RuntimeError( "metadata-client is required but not installed or failed to import. " f"Original error: {exc}. Install it or run: pip install -r dp-client/requirements.txt" ) from exc + self._client = client - self._users_create = _users_create - self._users_list = _users_list - self._users_retrieve = _users_retrieve self._User = _User + # Load all endpoints dynamically + self._ep: Dict[str, Any] = { + key: getattr(_users_api, attr_name, None) for key, attr_name in self._ENDPOINT_ATTRS.items() + } + + # Validate presence of ALL endpoints (include all options) + missing = [self._ENDPOINT_ATTRS[k] for k, v in self._ep.items() if v is None] + if missing: + raise RuntimeError( + "metadata_client.api.users is missing required endpoints: " + + ", ".join(missing) + + ". Regenerate the client from the updated API spec." + ) + def create_user(self, user: Union[Any, dict]): + """Create a user via POST /api/users/. + + Args: + user: Either a metadata_client.models.User instance or a dict that + can be converted to one via User.from_dict. + + Returns: + The detailed response object from the generated client containing + status_code, content, headers, and parsed payload. + """ body = self._User.from_dict(user) if isinstance(user, dict) else user - return self._users_create.sync_detailed(client=self._client, body=body) + return self._ep["create"].sync_detailed(client=self._client, body=body) def get_user(self, user_id: str): - return self._users_retrieve.sync_detailed(client=self._client, id=user_id) + """Retrieve a user by ID via GET /api/users/{id}/. + + Args: + user_id: The primary key of the user to retrieve. + + Returns: + The detailed response object from the generated client. + """ + return self._ep["retrieve"].sync_detailed(client=self._client, id=user_id) def list_users(self): - return self._users_list.sync_detailed(client=self._client) + """List all users via GET /api/users/. + + Returns: + The detailed response object from the generated client. + """ + return self._ep["list"].sync_detailed(client=self._client) + + def update_user(self, user_id: str, body: Union[Any, dict]): + """Update a user via PUT /api/users/{id}/. + + Args: + user_id: The primary key of the user to update. + body: Either a metadata_client.models.User (or compatible dict) with + fields to fully update. "id" must not be included by server rules. + + Returns: + The detailed response object from the generated client. + """ + body_obj = self._User.from_dict(body) if isinstance(body, dict) else body + return self._ep["update"].sync_detailed(client=self._client, id=user_id, body=body_obj) + + def partial_update_user(self, user_id: str, body: dict): + """Partially update a user via PATCH /api/users/{id}/. + + Args: + user_id: The primary key of the user to update. + body: A partial JSON dictionary of fields to update. Must not + include "id" (server enforces immutability). + + Returns: + The detailed response object from the generated client. + """ + # PATCH accepts a partial dict + return self._ep["partial_update"].sync_detailed(client=self._client, id=user_id, body=body) + + def delete_user(self, user_id: str): + """Delete a user via DELETE /api/users/{id}/. + + Args: + user_id: The primary key of the user to delete. + + Returns: + The detailed response object from the generated client. + """ + return self._ep["destroy"].sync_detailed(client=self._client, id=user_id) diff --git a/dp-client/dp_client/client.py b/dp-client/dp_client/client.py index 9a235e5..52f79b0 100644 --- a/dp-client/dp_client/client.py +++ b/dp-client/dp_client/client.py @@ -28,6 +28,14 @@ def __init__( prefix: str = "Bearer", timeout: float = 10.0, ) -> None: + """Initialize DPClient and its composed helpers. + + Args: + base_url: Base URL of the metadata server (e.g., http://localhost:8000/api). + token: Optional bearer token for authenticated requests. + prefix: Authorization prefix (default: "Bearer"). + timeout: Default request timeout in seconds for higher-level operations. + """ self._client_factory = MetaDataServerAPIClientFactory() self.MetaDataServerAPIClient: Any = self._client_factory.build(base_url=base_url, token=token, prefix=prefix) self.UsersApi = UsersAPI(self.MetaDataServerAPIClient) @@ -36,13 +44,75 @@ def __init__( self._timeout = timeout def health_check(self): + """Call the health check endpoint. + + Returns: + The detailed response from the generated client for GET /api/health/. + """ return self.HealthAPI.health_check() def create_user(self, user: Union[Any, dict[str, Any]]): + """Create a user. + + Args: + user: User model instance or dict payload for user creation. + + Returns: + The detailed response from the generated client. + """ return self.UsersApi.create_user(user) def get_user(self, user_id: str): + """Retrieve a user by ID. + + Args: + user_id: Primary key of the user. + + Returns: + The detailed response from the generated client. + """ return self.UsersApi.get_user(user_id) def list_users(self): + """List all users. + + Returns: + The detailed response from the generated client. + """ return self.UsersApi.list_users() + + # New endpoints wrappers + def update_user(self, user_id: str, body): + """Update a user via PUT /api/users/{id}/. + + Args: + user_id: Primary key of the user to update. + body: User model instance or compatible dict with fields to update. + + Returns: + The detailed response from the generated client. + """ + return self.UsersApi.update_user(user_id, body) + + def partial_update_user(self, user_id: str, body: dict): + """Partially update a user via PATCH /api/users/{id}/. + + Args: + user_id: Primary key of the user to update. + body: Partial dict of fields to update (must not include "id"). + + Returns: + The detailed response from the generated client. + """ + return self.UsersApi.partial_update_user(user_id, body) + + def delete_user(self, user_id: str): + """Delete a user via DELETE /api/users/{id}/. + + Args: + user_id: Primary key of the user to delete. + + Returns: + The detailed response from the generated client. + """ + return self.UsersApi.delete_user(user_id) diff --git a/src/api_spec_hash.md5 b/src/api_spec_hash.md5 index ae14077..9d2a875 100644 --- a/src/api_spec_hash.md5 +++ b/src/api_spec_hash.md5 @@ -1 +1 @@ -9d068b1e36949ce29ff74821e3f41950 +b430c5369031f6dcc42cb4224fcc8649 diff --git a/src/metadata_manager/serializers.py b/src/metadata_manager/serializers.py index 9bf6786..8239139 100644 --- a/src/metadata_manager/serializers.py +++ b/src/metadata_manager/serializers.py @@ -11,3 +11,37 @@ class UserSerializer(serializers.ModelSerializer): class Meta: model = User fields = ["id", "name", "phone", "address"] + + +class UserUpdateSerializer(serializers.ModelSerializer): + """Serializer for updating users enforcing id immutability. + + - `id` is read-only. + - If `id` appears in the request body and differs from the instance id, return 400. + - If `id` equals the existing instance id, allow it (some clients always send it). + """ + + class Meta: + model = User + fields = ["id", "name", "phone", "address"] + read_only_fields = ["id"] + + def validate(self, attrs): + """Validate update payload and enforce `id` immutability. + + Args: + attrs: The validated attributes collected by the serializer for update. + + Returns: + dict: The validated attributes to be used by the serializer. + + Raises: + serializers.ValidationError: If the input payload attempts to change + the immutable primary key `id`. + """ + incoming = getattr(self, "initial_data", {}) or {} + if "id" in incoming and self.instance is not None: + # Allow if the provided id equals the current instance id + if str(incoming.get("id")) != str(self.instance.id): + raise serializers.ValidationError({"id": "Updating id is not allowed."}) + return super().validate(attrs) diff --git a/src/metadata_manager/urls.py b/src/metadata_manager/urls.py index c4f42fd..f127680 100644 --- a/src/metadata_manager/urls.py +++ b/src/metadata_manager/urls.py @@ -2,7 +2,7 @@ Exposes: - /api/users/ (list, create) -- /api/users// (retrieve) +- /api/users// (retrieve, update, partial_update, destroy) - /api/health/ (public health check) Additionally registers routes with a DRF router for the UserViewSet. """ @@ -17,7 +17,17 @@ urlpatterns = [ path("users/", UserViewSet.as_view({"get": "list", "post": "create"})), - path("users//", UserViewSet.as_view({"get": "retrieve"})), + path( + "users//", + UserViewSet.as_view( + { + "get": "retrieve", + "put": "update", + "patch": "partial_update", + "delete": "destroy", + } + ), + ), path("health/", HealthCheck.as_view()), path("", include(router.urls)), ] diff --git a/src/metadata_manager/views.py b/src/metadata_manager/views.py index 4d5e446..23101e7 100644 --- a/src/metadata_manager/views.py +++ b/src/metadata_manager/views.py @@ -12,23 +12,36 @@ from rest_framework.response import Response from .models.user import User -from .serializers import UserSerializer +from .serializers import UserSerializer, UserUpdateSerializer class UserViewSet( mixins.CreateModelMixin, mixins.RetrieveModelMixin, mixins.ListModelMixin, + mixins.UpdateModelMixin, + mixins.DestroyModelMixin, viewsets.GenericViewSet, ): """User operations. - Provides create, retrieve, and list operations for User resources. + Provides create, retrieve, list, update, partial_update, and destroy. """ queryset = User.objects.all() serializer_class = UserSerializer + def get_serializer_class(self): + """Return the appropriate serializer class for the current action. + + Returns: + type: UserUpdateSerializer for update/partial_update actions, or + the default serializer class otherwise. + """ + if getattr(self, "action", None) in {"update", "partial_update"}: + return UserUpdateSerializer + return super().get_serializer_class() + @action(detail=False, methods=["get"]) def ids(self, request: Request) -> Response: """List all user IDs. diff --git a/src/open_api_spec.yml b/src/open_api_spec.yml index c9deb78..87fe7d8 100644 --- a/src/open_api_spec.yml +++ b/src/open_api_spec.yml @@ -223,7 +223,7 @@ paths: description: |- User operations. - Provides create, retrieve, and list operations for User resources. + Provides create, retrieve, list, update, partial_update, and destroy. tags: - users security: @@ -243,7 +243,7 @@ paths: description: |- User operations. - Provides create, retrieve, and list operations for User resources. + Provides create, retrieve, list, update, partial_update, and destroy. tags: - users requestBody: @@ -268,7 +268,7 @@ paths: description: |- User operations. - Provides create, retrieve, and list operations for User resources. + Provides create, retrieve, list, update, partial_update, and destroy. parameters: - in: path name: id @@ -287,6 +287,85 @@ paths: schema: $ref: '#/components/schemas/User' description: '' + put: + operationId: users_update + description: |- + User operations. + + Provides create, retrieve, list, update, partial_update, and destroy. + parameters: + - in: path + name: id + schema: + type: string + required: true + tags: + - users + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/UserUpdate' + required: true + security: + - bearerAuth: [] + - bearerAuth: [] + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/UserUpdate' + description: '' + patch: + operationId: users_partial_update + description: |- + User operations. + + Provides create, retrieve, list, update, partial_update, and destroy. + parameters: + - in: path + name: id + schema: + type: string + required: true + tags: + - users + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/PatchedUserUpdate' + security: + - bearerAuth: [] + - bearerAuth: [] + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/UserUpdate' + description: '' + delete: + operationId: users_destroy + description: |- + User operations. + + Provides create, retrieve, list, update, partial_update, and destroy. + parameters: + - in: path + name: id + schema: + type: string + required: true + tags: + - users + security: + - bearerAuth: [] + - bearerAuth: [] + responses: + '204': + description: No response body /api/users/ids/: get: operationId: users_ids_retrieve @@ -324,6 +403,30 @@ components: type: string required: - status + PatchedUserUpdate: + type: object + description: |- + Serializer for updating users with strict id immutability. + + - `id` is read-only. + - If `id` appears in the request body, return 400 with a clear error. + properties: + id: + type: string + readOnly: true + description: Israeli ID (string, 5-9 digits, valid checksum) + name: + type: string + description: Full name (required). + maxLength: 100 + phone: + type: string + description: Phone number in E.164 format (e.g., +972...). + maxLength: 20 + address: + type: string + description: Street address (required). + maxLength: 255 TokenObtainPair: type: object properties: @@ -381,6 +484,35 @@ components: - id - name - phone + UserUpdate: + type: object + description: |- + Serializer for updating users with strict id immutability. + + - `id` is read-only. + - If `id` appears in the request body, return 400 with a clear error. + properties: + id: + type: string + readOnly: true + description: Israeli ID (string, 5-9 digits, valid checksum) + name: + type: string + description: Full name (required). + maxLength: 100 + phone: + type: string + description: Phone number in E.164 format (e.g., +972...). + maxLength: 20 + address: + type: string + description: Street address (required). + maxLength: 255 + required: + - address + - id + - name + - phone securitySchemes: bearerAuth: type: http diff --git a/tests/component/test_user_model_component.py b/tests/component/test_user_model_component.py index cfe5e43..e73aa05 100644 --- a/tests/component/test_user_model_component.py +++ b/tests/component/test_user_model_component.py @@ -74,7 +74,6 @@ def created_user_ids(client: DPClient) -> Generator[List[str], None, None]: def test_health_check_component(non_authenticated_client: DPClient): response = non_authenticated_client.health_check() - assert response.status_code == 200 assert response.status_code == HTTPStatus.OK @@ -99,6 +98,11 @@ def test_create_user_valid_component(client: DPClient, created_user_ids, require assert db_user["phone"] == payload["phone"] assert db_user["address"] == payload["address"] + # Cleanup: delete via API and validate DB removal + del_resp = client.delete_user(payload["id"]) # 204 expected + assert del_resp.status_code == HTTPStatus.NO_CONTENT + assert client.PGDBClient.get_user_by_id(payload["id"]) is None + def test_create_user_invalid_id_component(client: DPClient, require_db): payload = { @@ -108,7 +112,6 @@ def test_create_user_invalid_id_component(client: DPClient, require_db): "address": "Test Street 1", } response = client.create_user(payload) - assert response.status_code == 400 assert response.status_code == HTTPStatus.BAD_REQUEST assert response.parsed is None @@ -124,7 +127,6 @@ def test_create_user_invalid_phone_component(client: DPClient, require_db): "address": "Test Street 1", } response = client.create_user(payload) - assert response.status_code == 400 assert response.status_code == HTTPStatus.BAD_REQUEST assert response.parsed is None @@ -152,6 +154,11 @@ def test_retrieve_user_component(client: DPClient, created_user_ids, require_db) assert response.parsed.id == payload["id"] assert response.parsed.name == payload["name"] + # Cleanup: delete and verify DB removal + del_resp = client.delete_user(payload["id"]) # 204 expected + assert del_resp.status_code == HTTPStatus.NO_CONTENT + assert client.PGDBClient.get_user_by_id(payload["id"]) is None + def test_list_users_component(client: DPClient, created_user_ids, require_db): users_data = [ @@ -183,3 +190,124 @@ def test_list_users_component(client: DPClient, created_user_ids, require_db): returned_ids_api = {u.id for u in list_resp.parsed} for u in users_data: assert u["id"] in returned_ids_api + + # Cleanup: delete all created users via API and validate DB removal + for u in users_data: + del_resp = client.delete_user(u["id"]) # 204 expected + assert del_resp.status_code == HTTPStatus.NO_CONTENT + assert client.PGDBClient.get_user_by_id(u["id"]) is None + + +# ---- Parametrized update/patch and delete with DB validations ---- + + +def _new_user_payload() -> dict: + return { + "id": generate_israeli_id(), + "name": "Comp A", + "phone": generate_random_phone_number(), + "address": "Addr A", + } + + +PATCH_CASES_COMPONENT = [ + pytest.param(lambda: {"address": "Comp St 99"}, True, id="component:patch-address:ok"), + pytest.param(lambda: {"name": "Comp Renamed"}, True, id="component:patch-name:ok"), + pytest.param(lambda: {"phone": generate_random_phone_number()}, True, id="component:patch-phone:ok"), + pytest.param(lambda: {"phone": "0501234567"}, False, id="component:patch-phone:bad-format"), + pytest.param(lambda: {"id": generate_israeli_id()}, False, id="component:patch-id:forbidden"), +] + + +@pytest.mark.usefixtures("require_db") +@pytest.mark.parametrize("payload_builder,expect_ok", PATCH_CASES_COMPONENT) +def test_component_users_partial_update_parametrized(client: DPClient, created_user_ids, payload_builder, expect_ok): + payload = _new_user_payload() + created_user_ids.append(payload["id"]) # ensure cleanup + + create_resp = client.create_user(payload) + assert create_resp.status_code == HTTPStatus.CREATED + + patch_body = payload_builder() + resp = client.partial_update_user(payload["id"], patch_body) + + if expect_ok: + assert resp.status_code == HTTPStatus.OK + for k, v in patch_body.items(): + if k == "id": + continue + assert getattr(resp.parsed, k) == v + # DB verify + db = client.PGDBClient.get_user_by_id(payload["id"]) + for k, v in patch_body.items(): + if k != "id": + assert db[k] == v + else: + assert resp.status_code == HTTPStatus.BAD_REQUEST + assert resp.parsed is None + # DB should remain unchanged + db = client.PGDBClient.get_user_by_id(payload["id"]) + assert db is not None + + # Cleanup: delete via API and validate DB removal + del_resp = client.delete_user(payload["id"]) # 204 expected + assert del_resp.status_code == HTTPStatus.NO_CONTENT + assert client.PGDBClient.get_user_by_id(payload["id"]) is None + + +PUT_CASES_COMPONENT = [ + pytest.param( + lambda: {"name": "Comp B", "phone": generate_random_phone_number(), "address": "Addr B"}, + True, + id="component:put-no-id:ok", + ), + pytest.param( + lambda: { + "id": generate_israeli_id(), + "name": "Comp C", + "phone": generate_random_phone_number(), + "address": "Addr C", + }, + False, + id="component:put-with-id:forbidden", + ), + pytest.param( + lambda: {"name": "Comp D", "phone": "0501234567", "address": "Addr D"}, + False, + id="component:put-invalid-phone:bad-format", + ), +] + + +@pytest.mark.usefixtures("require_db") +@pytest.mark.parametrize("payload_builder,expect_ok", PUT_CASES_COMPONENT) +def test_component_users_update_put_parametrized(client: DPClient, created_user_ids, payload_builder, expect_ok): + payload = _new_user_payload() + created_user_ids.append(payload["id"]) # ensure cleanup + + create_resp = client.create_user(payload) + assert create_resp.status_code == HTTPStatus.CREATED + + put_body = payload_builder() + resp = client.update_user(payload["id"], put_body) + + if expect_ok: + assert resp.status_code == HTTPStatus.OK + assert resp.parsed.id == payload["id"] + for k, v in put_body.items(): + if k == "id": + continue + assert getattr(resp.parsed, k) == v + # DB verify + db = client.PGDBClient.get_user_by_id(payload["id"]) + for k, v in put_body.items(): + if k != "id": + assert db[k] == v + else: + assert resp.status_code == HTTPStatus.BAD_REQUEST + assert resp.parsed is None + + # Cleanup: delete via API and validate DB removal + del_resp = client.delete_user(payload["id"]) # 204 expected + assert del_resp.status_code == HTTPStatus.NO_CONTENT + assert client.PGDBClient.get_user_by_id(payload["id"]) is None diff --git a/tests/conftest.py b/tests/conftest.py index 65e9c28..54cc9e5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,8 +2,15 @@ import os from pathlib import Path +from typing import Any import pytest +import requests +from metadata_client import AuthenticatedClient as _AuthenticatedClient +from metadata_client import Client as _Client +from metadata_client.models import User + +from src.tools import generate_israeli_id, generate_random_phone_number try: # Enable local hostname fallback for Postgres unless explicitly disabled @@ -20,11 +27,14 @@ pass -@pytest.fixture +@pytest.fixture(scope="session", autouse=True) def base_url() -> str: """Resolve API base URL from environment with sensible defaults. Prefers API_BASE_URL; otherwise builds from METADATA_HOST and METADATA_PORT. + + Returns: + str: A non-empty base URL string. Always truthy; falls back to http://localhost:8000. """ return os.environ.get( "API_BASE_URL", @@ -32,7 +42,7 @@ def base_url() -> str: ) -@pytest.fixture +@pytest.fixture(scope="session", autouse=True) def auth_token(base_url: str) -> str: """Get an access token for authenticated API tests. @@ -42,8 +52,6 @@ def auth_token(base_url: str) -> str: if token: return token - import requests # imported here to avoid hard dependency if not needed - username = os.environ.get("TEST_USERNAME") password = os.environ.get("TEST_PASSWORD") if not username or not password: @@ -56,3 +64,57 @@ def auth_token(base_url: str) -> str: ) resp.raise_for_status() return resp.json()["access"] + + +try: + AuthenticatedClient: Any = _AuthenticatedClient + Client: Any = _Client +except Exception: + AuthenticatedClient = None + Client = None + + +@pytest.fixture(scope="session", autouse=True) +def non_authenticated_client(base_url: str): + """Unauthenticated generated API client (metadata_client.Client). + + Args: + base_url: Base URL for the API under test. + + Returns: + metadata_client.Client instance. + """ + if Client is None: + raise RuntimeError("metadata_client is not installed. Generate or install the client before running tests.") + return Client(base_url=base_url) + + +@pytest.fixture(scope="session", autouse=True) +def client(base_url: str, auth_token: str): + """Authenticated generated API client (metadata_client.AuthenticatedClient). + + Args: + base_url: Base URL for the API under test. + auth_token: Access token for Authorization header. + + Returns: + metadata_client.AuthenticatedClient instance. + """ + if AuthenticatedClient is None: + raise RuntimeError("metadata_client is not installed. Generate or install the client before running tests.") + return AuthenticatedClient(base_url=base_url, token=auth_token, prefix="Bearer") + + +@pytest.fixture(scope="function") +def new_user() -> User: + """Build a fresh valid User payload for tests. + + Returns: + User: A new User instance with randomized, valid fields. + """ + return User( + id=generate_israeli_id(), + name="User A", + phone=generate_random_phone_number(), + address="Addr A", + ) diff --git a/tests/unit/test_user_model.py b/tests/unit/test_user_model.py index a082564..0e79500 100644 --- a/tests/unit/test_user_model.py +++ b/tests/unit/test_user_model.py @@ -1,82 +1,150 @@ from http import HTTPStatus import pytest -from metadata_client import AuthenticatedClient, Client +from metadata_client import AuthenticatedClient from metadata_client.api.health import health_retrieve from metadata_client.api.users import ( users_create, - users_list, + users_destroy, + users_partial_update, users_retrieve, + users_update, ) -from metadata_client.models import User +from metadata_client.models import User, UserUpdate, PatchedUserUpdate from src.tools import generate_israeli_id, generate_random_phone_number -@pytest.fixture -def non_authenticated_client(base_url: str): - return Client(base_url=base_url) - - -@pytest.fixture -def client(base_url: str, auth_token: str): - return AuthenticatedClient(base_url=base_url, token=auth_token, prefix="Bearer") - - def test_health_check(non_authenticated_client): response = health_retrieve.sync_detailed(client=non_authenticated_client) - assert response.status_code == 200 assert response.status_code == HTTPStatus.OK -def test_create_user_valid(client): - user = User( - id=generate_israeli_id(), name="Test User", phone=generate_random_phone_number(), address="Test Street 1" - ) - response = users_create.sync_detailed(client=client, body=user) - assert response.status_code == HTTPStatus.CREATED - assert response.parsed.id == user.id - assert response.parsed.name == user.name - - -def test_create_user_invalid_id(client): - user = User(id="123789456", name="Test User", phone=generate_random_phone_number(), address="Test Street 1") - response = users_create.sync_detailed(client=client, body=user) - assert response.status_code == 400 - assert response.status_code == HTTPStatus.BAD_REQUEST - assert response.parsed is None - - -def test_create_user_invalid_phone(client): - user = User(id=generate_israeli_id(), name="Test User", phone="0501234567", address="Test Street 1") - response = users_create.sync_detailed(client=client, body=user) - assert response.status_code == 400 - assert response.status_code == HTTPStatus.BAD_REQUEST - assert response.parsed is None - - -def test_retrieve_user(client): - user = User( - id=generate_israeli_id(), name="Test User", phone=generate_random_phone_number(), address="Test Street 1" - ) +CREATE_CASES = [ + pytest.param( + lambda: User(id=generate_israeli_id(), name="Ok", phone=generate_random_phone_number(), address="A"), + True, + id="create:ok", + ), + pytest.param( + lambda: User(id="123789456", name="BadID", phone=generate_random_phone_number(), address="A"), + False, + id="create:bad-id", + ), + pytest.param( + lambda: User(id=generate_israeli_id(), name="BadPhone", phone="0501234567", address="A"), + False, + id="create:bad-phone", + ), +] + + +@pytest.mark.parametrize("user_builder,expect_ok", CREATE_CASES) +def test_users_create_parametrized(client, user_builder, expect_ok): + user = user_builder() + resp = users_create.sync_detailed(client=client, body=user) + if expect_ok: + assert resp.status_code == HTTPStatus.CREATED + assert resp.parsed.id == user.id + # Cleanup: delete via API and validate removal + del_resp = users_destroy.sync_detailed(client=client, id=user.id) + assert del_resp.status_code == HTTPStatus.NO_CONTENT + get_resp = users_retrieve.sync_detailed(client=client, id=user.id) + assert get_resp.status_code == HTTPStatus.NOT_FOUND + else: + assert resp.status_code == HTTPStatus.BAD_REQUEST + assert resp.parsed is None + + +PATCH_CASES = [ + pytest.param(lambda: {"address": "New Street 1"}, True, id="patch-address:ok"), + pytest.param(lambda: {"name": "Renamed"}, True, id="patch-name:ok"), + pytest.param(lambda: {"phone": generate_random_phone_number()}, True, id="patch-phone:ok-valid"), + pytest.param(lambda: {"phone": "0501234567"}, False, id="patch-phone:bad-format"), + pytest.param(lambda: {"id": generate_israeli_id()}, False, id="patch-id:forbidden"), +] + + +@pytest.mark.parametrize("payload_builder,expect_ok", PATCH_CASES) +def test_users_partial_update_parametrized(client: AuthenticatedClient, payload_builder, expect_ok, new_user: User): + user = new_user create_resp = users_create.sync_detailed(client=client, body=user) assert create_resp.status_code == HTTPStatus.CREATED - response = users_retrieve.sync_detailed(client=client, id=user.id) - assert response.status_code == HTTPStatus.OK - assert response.parsed.id == user.id - assert response.parsed.name == user.name + patch_body = payload_builder() + m_body = PatchedUserUpdate.from_dict(patch_body) + resp = users_partial_update.sync_detailed(client=client, id=user.id, body=m_body) + + if expect_ok: + assert resp.status_code == HTTPStatus.OK + assert resp.parsed is not None + for k, v in patch_body.items(): + if k == "id": + continue + assert getattr(resp.parsed, k) == v + assert resp.parsed.id == user.id + else: + assert resp.status_code == HTTPStatus.BAD_REQUEST + assert resp.parsed is None + + # Cleanup: delete the created user and validate deletion + del_resp = users_destroy.sync_detailed(client=client, id=user.id) + assert del_resp.status_code == HTTPStatus.NO_CONTENT + get_resp = users_retrieve.sync_detailed(client=client, id=user.id) + assert get_resp.status_code == HTTPStatus.NOT_FOUND + + +PUT_CASES = [ + pytest.param( + # OK: client sends id equal to path (many generators require id in model) + lambda cur_id: {"id": cur_id, "name": "User B", "phone": generate_random_phone_number(), "address": "Addr B"}, + True, + id="put-with-same-id:ok", + ), + pytest.param( + # Forbidden: client sends a different id attempting to change PK + lambda cur_id: { + "id": generate_israeli_id(), + "name": "User B", + "phone": generate_random_phone_number(), + "address": "Addr B", + }, + False, + id="put-with-different-id:forbidden", + ), + pytest.param( + # Invalid phone while keeping same id + lambda cur_id: {"id": cur_id, "name": "User C", "phone": "0501234567", "address": "Addr C"}, + False, + id="put-invalid-phone:bad-format", + ), +] + + +@pytest.mark.parametrize("payload_builder,expect_ok", PUT_CASES) +def test_users_update_put_parametrized(client: AuthenticatedClient, payload_builder, expect_ok, new_user: User): + user = new_user + create_resp = users_create.sync_detailed(client=client, body=user) + assert create_resp.status_code == HTTPStatus.CREATED -def test_list_users(client): - users_data = [ - User(id=generate_israeli_id(), name="Test User", phone=generate_random_phone_number(), address="Street 1"), - User(id=generate_israeli_id(), name="Test User", phone=generate_random_phone_number(), address="Street 2"), - ] - for user in users_data: - resp = users_create.sync_detailed(client=client, body=user) - assert resp.status_code == HTTPStatus.CREATED - list_resp = users_list.sync_detailed(client=client) - assert list_resp.status_code == HTTPStatus.OK - returned_ids = {u.id for u in list_resp.parsed} - for user in users_data: - assert user.id in returned_ids + put_body = payload_builder(user.id) + m_body = UserUpdate.from_dict(put_body) + resp = users_update.sync_detailed(client=client, id=user.id, body=m_body) + + if expect_ok: + assert resp.status_code == HTTPStatus.OK + assert resp.parsed is not None + assert resp.parsed.id == user.id + for k, v in put_body.items(): + if k == "id": + continue + assert getattr(resp.parsed, k) == v + else: + assert resp.status_code == HTTPStatus.BAD_REQUEST + assert resp.parsed is None + + # Cleanup: delete the created user and validate deletion + del_resp = users_destroy.sync_detailed(client=client, id=user.id) + assert del_resp.status_code == HTTPStatus.NO_CONTENT + get_resp = users_retrieve.sync_detailed(client=client, id=user.id) + assert get_resp.status_code == HTTPStatus.NOT_FOUND From 8aebac8bd409896640a5be4f94be876b517f737a Mon Sep 17 00:00:00 2001 From: sidkos Date: Sun, 5 Oct 2025 16:42:52 +0300 Subject: [PATCH 2/7] wip2 --- dp-client/dp_client/api/users.py | 44 ++++++++++++++++++++++++-------- mypy.ini | 2 +- scripts/precommit.sh | 2 +- src/api_spec_hash.md5 | 2 +- src/open_api_spec.yml | 10 +++++--- tests/conftest.py | 7 +++++ tests/unit/test_user_model.py | 2 +- 7 files changed, 50 insertions(+), 19 deletions(-) diff --git a/dp-client/dp_client/api/users.py b/dp-client/dp_client/api/users.py index d098135..1ed6abd 100644 --- a/dp-client/dp_client/api/users.py +++ b/dp-client/dp_client/api/users.py @@ -34,7 +34,9 @@ def __init__(self, client: Any) -> None: """ try: from metadata_client.api import users as _users_api + from metadata_client.models import PatchedUserUpdate as _PatchedUserUpdate from metadata_client.models import User as _User + from metadata_client.models import UserUpdate as _UserUpdate except Exception as exc: raise RuntimeError( "metadata-client is required but not installed or failed to import. " @@ -43,11 +45,21 @@ def __init__(self, client: Any) -> None: self._client = client self._User = _User - - # Load all endpoints dynamically - self._ep: Dict[str, Any] = { - key: getattr(_users_api, attr_name, None) for key, attr_name in self._ENDPOINT_ATTRS.items() - } + self._UserUpdate = _UserUpdate + self._PatchedUserUpdate = _PatchedUserUpdate + + # Load all endpoints by importing submodules explicitly since + # metadata_client.api.users may not expose submodules as attributes. + import importlib + + self._ep: Dict[str, Any] = {} + for key, attr_name in self._ENDPOINT_ATTRS.items(): + module_name = f"metadata_client.api.users.{attr_name}" + try: + mod = importlib.import_module(module_name) + except Exception: + mod = None + self._ep[key] = mod # Validate presence of ALL endpoints (include all options) missing = [self._ENDPOINT_ATTRS[k] for k, v in self._ep.items() if v is None] @@ -96,13 +108,22 @@ def update_user(self, user_id: str, body: Union[Any, dict]): Args: user_id: The primary key of the user to update. - body: Either a metadata_client.models.User (or compatible dict) with - fields to fully update. "id" must not be included by server rules. + body: Either a metadata_client.models.UserUpdate instance or a dict + that will be converted with UserUpdate.from_dict. If the dict + omits an "id" field, this client will inject the path id to + satisfy the generated model requirements. Attempting to change + the id (mismatch between body and path) will result in 400 from + the server. Returns: The detailed response object from the generated client. """ - body_obj = self._User.from_dict(body) if isinstance(body, dict) else body + if isinstance(body, dict): + if "id" not in body: + body = {**body, "id": user_id} + body_obj = self._UserUpdate.from_dict(body) + else: + body_obj = body return self._ep["update"].sync_detailed(client=self._client, id=user_id, body=body_obj) def partial_update_user(self, user_id: str, body: dict): @@ -111,13 +132,14 @@ def partial_update_user(self, user_id: str, body: dict): Args: user_id: The primary key of the user to update. body: A partial JSON dictionary of fields to update. Must not - include "id" (server enforces immutability). + include "id" (server enforces immutability). Converted to + PatchedUserUpdate model for the generated client. Returns: The detailed response object from the generated client. """ - # PATCH accepts a partial dict - return self._ep["partial_update"].sync_detailed(client=self._client, id=user_id, body=body) + m_body = self._PatchedUserUpdate.from_dict(body) + return self._ep["partial_update"].sync_detailed(client=self._client, id=user_id, body=m_body) def delete_user(self, user_id: str): """Delete a user via DELETE /api/users/{id}/. diff --git a/mypy.ini b/mypy.ini index a467363..ca098e2 100644 --- a/mypy.ini +++ b/mypy.ini @@ -5,7 +5,7 @@ warn_unused_ignores = True warn_unreachable = True allow_redefinition = True warn_unused_configs = True -strict = False +strict = True plugins = mypy_django_plugin.main explicit_package_bases = True namespace_packages = True diff --git a/scripts/precommit.sh b/scripts/precommit.sh index 0ba562a..117ab20 100755 --- a/scripts/precommit.sh +++ b/scripts/precommit.sh @@ -7,5 +7,5 @@ black . flake8 . pycodestyle . # Let mypy read targets from mypy.ini (avoids duplicate module names like src.package vs package) -PYTHONPATH=src mypy +mypy . yamllint . --no-warnings \ No newline at end of file diff --git a/src/api_spec_hash.md5 b/src/api_spec_hash.md5 index 9d2a875..56d9ca7 100644 --- a/src/api_spec_hash.md5 +++ b/src/api_spec_hash.md5 @@ -1 +1 @@ -b430c5369031f6dcc42cb4224fcc8649 +65b9d4481f9132a8228ddf7cacf649de diff --git a/src/open_api_spec.yml b/src/open_api_spec.yml index 87fe7d8..c4d8fa1 100644 --- a/src/open_api_spec.yml +++ b/src/open_api_spec.yml @@ -406,10 +406,11 @@ components: PatchedUserUpdate: type: object description: |- - Serializer for updating users with strict id immutability. + Serializer for updating users enforcing id immutability. - `id` is read-only. - - If `id` appears in the request body, return 400 with a clear error. + - If `id` appears in the request body and differs from the instance id, return 400. + - If `id` equals the existing instance id, allow it (some clients always send it). properties: id: type: string @@ -487,10 +488,11 @@ components: UserUpdate: type: object description: |- - Serializer for updating users with strict id immutability. + Serializer for updating users enforcing id immutability. - `id` is read-only. - - If `id` appears in the request body, return 400 with a clear error. + - If `id` appears in the request body and differs from the instance id, return 400. + - If `id` equals the existing instance id, allow it (some clients always send it). properties: id: type: string diff --git a/tests/conftest.py b/tests/conftest.py index 54cc9e5..d7b27eb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,7 @@ from __future__ import annotations import os +import sys from pathlib import Path from typing import Any @@ -12,6 +13,12 @@ from src.tools import generate_israeli_id, generate_random_phone_number +# Ensure in-repo dp-client package is importable before site-packages version +_repo_root = Path(__file__).resolve().parents[1] +_dp_pkg_path = str(_repo_root / "dp-client") +if _dp_pkg_path not in sys.path: + sys.path.insert(0, _dp_pkg_path) + try: # Enable local hostname fallback for Postgres unless explicitly disabled # This lets component tests run locally when POSTGRES_HOST (e.g., 'db') isn't resolvable. diff --git a/tests/unit/test_user_model.py b/tests/unit/test_user_model.py index 0e79500..9518596 100644 --- a/tests/unit/test_user_model.py +++ b/tests/unit/test_user_model.py @@ -10,7 +10,7 @@ users_retrieve, users_update, ) -from metadata_client.models import User, UserUpdate, PatchedUserUpdate +from metadata_client.models import PatchedUserUpdate, User, UserUpdate from src.tools import generate_israeli_id, generate_random_phone_number From f84ece8d57c5e906050f6fc5d9aa4fb3b298de21 Mon Sep 17 00:00:00 2001 From: sidkos Date: Sun, 5 Oct 2025 17:49:50 +0300 Subject: [PATCH 3/7] wip3 --- dp-client/dp_client/__init__.py | 9 ++ dp-client/dp_client/api/health.py | 11 +- dp-client/dp_client/api/users.py | 54 ++++++--- dp-client/dp_client/client.py | 26 ++-- dp-client/dp_client/clients/metadata.py | 8 +- dp-client/dp_client/db/drivers/postgres.py | 21 +++- dp-client/dp_client/db/pg.py | 2 +- tests/component/test_user_model_component.py | 120 ++++++++++--------- tests/conftest.py | 17 ++- tests/unit/test_user_model.py | 27 +++-- 10 files changed, 176 insertions(+), 119 deletions(-) diff --git a/dp-client/dp_client/__init__.py b/dp-client/dp_client/__init__.py index 470e894..6be11c5 100644 --- a/dp-client/dp_client/__init__.py +++ b/dp-client/dp_client/__init__.py @@ -9,3 +9,12 @@ from .client import DPClient # noqa: F401 from .clients import MetaDataServerAPIClientFactory # noqa: F401 from .db import PGDBClient # noqa: F401 + +__all__ = [ + "__version__", + "DPClient", + "HealthAPI", + "UsersAPI", + "MetaDataServerAPIClientFactory", + "PGDBClient", +] diff --git a/dp-client/dp_client/api/health.py b/dp-client/dp_client/api/health.py index a7bc568..ce1f24d 100644 --- a/dp-client/dp_client/api/health.py +++ b/dp-client/dp_client/api/health.py @@ -1,12 +1,15 @@ from __future__ import annotations -from typing import Any +from typing import Union, cast + +from metadata_client import AuthenticatedClient, Client +from metadata_client.types import Response class HealthAPI: """Health endpoints wrapper using generated metadata_client api module.""" - def __init__(self, client: Any) -> None: + def __init__(self, client: Union[Client, AuthenticatedClient]) -> None: """Initialize the HealthAPI wrapper. Args: @@ -26,10 +29,10 @@ def __init__(self, client: Any) -> None: self._client = client self._health_retrieve = _health_retrieve - def health_check(self): + def health_check(self) -> Response[dict[str, str]]: """Call GET /api/health/ and return the detailed response. Returns: The detailed response object from the generated client. """ - return self._health_retrieve.sync_detailed(client=self._client) + return cast(Response[dict[str, str]], self._health_retrieve.sync_detailed(client=self._client)) diff --git a/dp-client/dp_client/api/users.py b/dp-client/dp_client/api/users.py index 1ed6abd..4f3823a 100644 --- a/dp-client/dp_client/api/users.py +++ b/dp-client/dp_client/api/users.py @@ -1,6 +1,14 @@ from __future__ import annotations -from typing import Any, Dict, Union +from types import ModuleType +from typing import TYPE_CHECKING, Dict, Union + +from metadata_client import AuthenticatedClient, Client +from metadata_client.types import Response + +if TYPE_CHECKING: # import only for type checkers; runtime uses lazy-loaded attributes + from metadata_client.models import User as _User + from metadata_client.models import UserUpdate as _UserUpdate class UsersAPI: @@ -20,7 +28,7 @@ class UsersAPI: "destroy": "users_destroy", } - def __init__(self, client: Any) -> None: + def __init__(self, client: Union[Client, AuthenticatedClient]) -> None: """Initialize the UsersAPI wrapper. Args: @@ -33,7 +41,6 @@ def __init__(self, client: Any) -> None: missing from metadata_client.api.users. """ try: - from metadata_client.api import users as _users_api from metadata_client.models import PatchedUserUpdate as _PatchedUserUpdate from metadata_client.models import User as _User from metadata_client.models import UserUpdate as _UserUpdate @@ -52,25 +59,29 @@ def __init__(self, client: Any) -> None: # metadata_client.api.users may not expose submodules as attributes. import importlib - self._ep: Dict[str, Any] = {} + tmp_ep: Dict[str, ModuleType | None] = {} for key, attr_name in self._ENDPOINT_ATTRS.items(): module_name = f"metadata_client.api.users.{attr_name}" try: mod = importlib.import_module(module_name) except Exception: mod = None - self._ep[key] = mod + tmp_ep[key] = mod # Validate presence of ALL endpoints (include all options) - missing = [self._ENDPOINT_ATTRS[k] for k, v in self._ep.items() if v is None] + missing = [self._ENDPOINT_ATTRS[k] for k, v in tmp_ep.items() if v is None] if missing: raise RuntimeError( "metadata_client.api.users is missing required endpoints: " + ", ".join(missing) + ". Regenerate the client from the updated API spec." ) + # Cast to non-optional after validation + from typing import cast - def create_user(self, user: Union[Any, dict]): + self._ep: Dict[str, ModuleType] = cast(Dict[str, ModuleType], tmp_ep) + + def create_user(self, user: Union[_User, dict[str, object]]) -> Response[_User]: """Create a user via POST /api/users/. Args: @@ -82,9 +93,10 @@ def create_user(self, user: Union[Any, dict]): status_code, content, headers, and parsed payload. """ body = self._User.from_dict(user) if isinstance(user, dict) else user - return self._ep["create"].sync_detailed(client=self._client, body=body) + res: Response[_User] = self._ep["create"].sync_detailed(client=self._client, body=body) + return res - def get_user(self, user_id: str): + def get_user(self, user_id: str) -> Response[_User]: """Retrieve a user by ID via GET /api/users/{id}/. Args: @@ -93,17 +105,19 @@ def get_user(self, user_id: str): Returns: The detailed response object from the generated client. """ - return self._ep["retrieve"].sync_detailed(client=self._client, id=user_id) + res: Response[_User] = self._ep["retrieve"].sync_detailed(client=self._client, id=user_id) + return res - def list_users(self): + def list_users(self) -> Response[list[_User]]: """List all users via GET /api/users/. Returns: The detailed response object from the generated client. """ - return self._ep["list"].sync_detailed(client=self._client) + res: Response[list[_User]] = self._ep["list"].sync_detailed(client=self._client) + return res - def update_user(self, user_id: str, body: Union[Any, dict]): + def update_user(self, user_id: str, body: Union[_UserUpdate, dict[str, object]]) -> Response[_User]: """Update a user via PUT /api/users/{id}/. Args: @@ -124,9 +138,11 @@ def update_user(self, user_id: str, body: Union[Any, dict]): body_obj = self._UserUpdate.from_dict(body) else: body_obj = body - return self._ep["update"].sync_detailed(client=self._client, id=user_id, body=body_obj) - def partial_update_user(self, user_id: str, body: dict): + res: Response[_User] = self._ep["update"].sync_detailed(client=self._client, id=user_id, body=body_obj) + return res + + def partial_update_user(self, user_id: str, body: dict[str, object]) -> Response[_User]: """Partially update a user via PATCH /api/users/{id}/. Args: @@ -139,9 +155,10 @@ def partial_update_user(self, user_id: str, body: dict): The detailed response object from the generated client. """ m_body = self._PatchedUserUpdate.from_dict(body) - return self._ep["partial_update"].sync_detailed(client=self._client, id=user_id, body=m_body) + res: Response[_User] = self._ep["partial_update"].sync_detailed(client=self._client, id=user_id, body=m_body) + return res - def delete_user(self, user_id: str): + def delete_user(self, user_id: str) -> Response[None]: """Delete a user via DELETE /api/users/{id}/. Args: @@ -150,4 +167,5 @@ def delete_user(self, user_id: str): Returns: The detailed response object from the generated client. """ - return self._ep["destroy"].sync_detailed(client=self._client, id=user_id) + res: Response[None] = self._ep["destroy"].sync_detailed(client=self._client, id=user_id) + return res diff --git a/dp-client/dp_client/client.py b/dp-client/dp_client/client.py index 52f79b0..a0bc912 100644 --- a/dp-client/dp_client/client.py +++ b/dp-client/dp_client/client.py @@ -1,6 +1,11 @@ from __future__ import annotations -from typing import Any, Optional, Union +from typing import Optional, Union + +from metadata_client import AuthenticatedClient, Client +from metadata_client.models import User as _User +from metadata_client.models import UserUpdate as _UserUpdate +from metadata_client.types import Response from .api.health import HealthAPI from .api.users import UsersAPI @@ -37,13 +42,15 @@ def __init__( timeout: Default request timeout in seconds for higher-level operations. """ self._client_factory = MetaDataServerAPIClientFactory() - self.MetaDataServerAPIClient: Any = self._client_factory.build(base_url=base_url, token=token, prefix=prefix) + self.MetaDataServerAPIClient: Union[Client, AuthenticatedClient] = self._client_factory.build( + base_url=base_url, token=token, prefix=prefix + ) self.UsersApi = UsersAPI(self.MetaDataServerAPIClient) self.HealthAPI = HealthAPI(self.MetaDataServerAPIClient) self.PGDBClient = PGDBClient() self._timeout = timeout - def health_check(self): + def health_check(self) -> Response[dict[str, str]]: """Call the health check endpoint. Returns: @@ -51,7 +58,7 @@ def health_check(self): """ return self.HealthAPI.health_check() - def create_user(self, user: Union[Any, dict[str, Any]]): + def create_user(self, user: Union[_User, dict[str, object]]) -> Response[_User]: """Create a user. Args: @@ -62,7 +69,7 @@ def create_user(self, user: Union[Any, dict[str, Any]]): """ return self.UsersApi.create_user(user) - def get_user(self, user_id: str): + def get_user(self, user_id: str) -> Response[_User]: """Retrieve a user by ID. Args: @@ -73,7 +80,7 @@ def get_user(self, user_id: str): """ return self.UsersApi.get_user(user_id) - def list_users(self): + def list_users(self) -> Response[list[_User]]: """List all users. Returns: @@ -82,7 +89,7 @@ def list_users(self): return self.UsersApi.list_users() # New endpoints wrappers - def update_user(self, user_id: str, body): + def update_user(self, user_id: str, body: Union[_UserUpdate, dict[str, object]]) -> Response[_User]: """Update a user via PUT /api/users/{id}/. Args: @@ -92,9 +99,10 @@ def update_user(self, user_id: str, body): Returns: The detailed response from the generated client. """ + # Delegate to UsersAPI which normalizes payload and types return self.UsersApi.update_user(user_id, body) - def partial_update_user(self, user_id: str, body: dict): + def partial_update_user(self, user_id: str, body: dict[str, object]) -> Response[_User]: """Partially update a user via PATCH /api/users/{id}/. Args: @@ -106,7 +114,7 @@ def partial_update_user(self, user_id: str, body: dict): """ return self.UsersApi.partial_update_user(user_id, body) - def delete_user(self, user_id: str): + def delete_user(self, user_id: str) -> Response[None]: """Delete a user via DELETE /api/users/{id}/. Args: diff --git a/dp-client/dp_client/clients/metadata.py b/dp-client/dp_client/clients/metadata.py index 49dc7fa..9633374 100644 --- a/dp-client/dp_client/clients/metadata.py +++ b/dp-client/dp_client/clients/metadata.py @@ -1,6 +1,8 @@ from __future__ import annotations -from typing import Any, Optional +from typing import Optional, Union + +from metadata_client import AuthenticatedClient, Client class MetaDataServerAPIClientFactory: @@ -22,7 +24,9 @@ def __init__(self) -> None: self._AuthenticatedClient = _AuthenticatedClient self._Client = _Client - def build(self, base_url: str, token: Optional[str] = None, prefix: str = "Bearer") -> Any: + def build( + self, base_url: str, token: Optional[str] = None, prefix: str = "Bearer" + ) -> Union[Client, AuthenticatedClient]: if token: return self._AuthenticatedClient(base_url=base_url, token=token, prefix=prefix) return self._Client(base_url=base_url) diff --git a/dp-client/dp_client/db/drivers/postgres.py b/dp-client/dp_client/db/drivers/postgres.py index 9d6a01b..18c141b 100644 --- a/dp-client/dp_client/db/drivers/postgres.py +++ b/dp-client/dp_client/db/drivers/postgres.py @@ -1,6 +1,8 @@ from __future__ import annotations -from typing import Any, Optional, Tuple +from typing import Any, Optional, Tuple, cast + +from psycopg2.extensions import connection as PGConnection try: import psycopg2 @@ -27,18 +29,25 @@ def __init__( ) -> None: self._conn_params = dict(host=host, port=port, dbname=dbname, user=user, password=password) - def _conn(self): + def _conn(self) -> PGConnection: return psycopg2.connect(**self._conn_params) - def fetch_one(self, query: str, params: Optional[Tuple[Any, ...]] = None) -> Optional[tuple]: + def fetch_one(self, query: str, params: Optional[Tuple[Any, ...]] = None) -> Optional[tuple[Any, ...]]: with self._conn() as conn: with conn.cursor() as cur: cur.execute(query, params or ()) - return cur.fetchone() + row = cast(Optional[tuple[Any, ...]], cur.fetchone()) + return row - def fetch_value(self, query: str, params: Optional[Tuple[Any, ...]] = None) -> Optional[Any]: + def fetch_value(self, query: str, params: Optional[Tuple[Any, ...]] = None) -> Optional[int]: row = self.fetch_one(query, params) - return row[0] if row is not None and len(row) > 0 else None + if row is None or len(row) == 0: + return None + try: + return int(row[0]) + except (TypeError, ValueError): + # Fallback: not an int-compatible value + return None def execute(self, query: str, params: Optional[Tuple[Any, ...]] = None) -> int: with self._conn() as conn: diff --git a/dp-client/dp_client/db/pg.py b/dp-client/dp_client/db/pg.py index c63f552..e0a53dc 100644 --- a/dp-client/dp_client/db/pg.py +++ b/dp-client/dp_client/db/pg.py @@ -90,7 +90,7 @@ def __init__( ) self._table = table - def get_user_by_id(self, user_id: str): + def get_user_by_id(self, user_id: str) -> Optional[dict[str, str]]: row = self._driver.fetch_one( f"SELECT id, name, phone, address FROM {self._table} WHERE id = %s", (user_id,), diff --git a/tests/component/test_user_model_component.py b/tests/component/test_user_model_component.py index e73aa05..4b4dfa5 100644 --- a/tests/component/test_user_model_component.py +++ b/tests/component/test_user_model_component.py @@ -1,5 +1,5 @@ from http import HTTPStatus -from typing import Generator, List +from typing import Callable, Dict, Generator, List import pytest from dp_client import DPClient @@ -8,7 +8,6 @@ # Define a tuple of psycopg2-related exceptions if psycopg2 is available. # This lets us catch specific DB connectivity/driver errors without a blanket Exception. -# Always keep a consistent, typed value to satisfy mypy. PSYCOPG2_ERRORS: tuple[type[BaseException], ...] = tuple() try: import psycopg2 @@ -34,7 +33,7 @@ def client(base_url: str, auth_token: str) -> DPClient: @pytest.fixture -def require_db(client: DPClient): +def require_db(client: DPClient) -> None: """Skip tests that require direct DB access if the DB is not reachable. Tries a simple query via PGDBClient; on failure, skips the test. @@ -72,41 +71,40 @@ def created_user_ids(client: DPClient) -> Generator[List[str], None, None]: pass -def test_health_check_component(non_authenticated_client: DPClient): +def test_health_check_component(non_authenticated_client: DPClient) -> None: response = non_authenticated_client.health_check() assert response.status_code == HTTPStatus.OK -def test_create_user_valid_component(client: DPClient, created_user_ids, require_db): - payload = { +def test_create_user_valid_component(client: DPClient, created_user_ids: List[str], require_db: None) -> None: + payload: Dict[str, object] = { "id": generate_israeli_id(), "name": "Test User", "phone": generate_random_phone_number(), "address": "Test Street 1", } - created_user_ids.append(payload["id"]) + created_user_ids.append(str(payload["id"])) response = client.create_user(payload) assert response.status_code == HTTPStatus.CREATED + assert response.parsed is not None assert response.parsed.id == payload["id"] assert response.parsed.name == payload["name"] - # Verify in DB via dp-client DB driver - db_user = client.PGDBClient.get_user_by_id(payload["id"]) + db_user = client.PGDBClient.get_user_by_id(str(payload["id"])) assert db_user is not None, "User should exist in DB after creation" assert db_user["name"] == payload["name"] assert db_user["phone"] == payload["phone"] assert db_user["address"] == payload["address"] - # Cleanup: delete via API and validate DB removal - del_resp = client.delete_user(payload["id"]) # 204 expected + del_resp = client.delete_user(str(payload["id"])) # 204 expected assert del_resp.status_code == HTTPStatus.NO_CONTENT - assert client.PGDBClient.get_user_by_id(payload["id"]) is None + assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None -def test_create_user_invalid_id_component(client: DPClient, require_db): - payload = { - "id": "123789456", # invalid checksum +def test_create_user_invalid_id_component(client: DPClient, require_db: None) -> None: + payload: Dict[str, object] = { + "id": "123789456", "name": "Test User", "phone": generate_random_phone_number(), "address": "Test Street 1", @@ -115,53 +113,50 @@ def test_create_user_invalid_id_component(client: DPClient, require_db): assert response.status_code == HTTPStatus.BAD_REQUEST assert response.parsed is None - # Verify not in DB - assert client.PGDBClient.get_user_by_id(payload["id"]) is None + assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None -def test_create_user_invalid_phone_component(client: DPClient, require_db): - payload = { +def test_create_user_invalid_phone_component(client: DPClient, require_db: None) -> None: + payload: Dict[str, object] = { "id": generate_israeli_id(), "name": "Test User", - "phone": "0501234567", # invalid format + "phone": "0501234567", "address": "Test Street 1", } response = client.create_user(payload) assert response.status_code == HTTPStatus.BAD_REQUEST assert response.parsed is None - # Verify not in DB - assert client.PGDBClient.get_user_by_id(payload["id"]) is None + assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None -def test_retrieve_user_component(client: DPClient, created_user_ids, require_db): - payload = { +def test_retrieve_user_component(client: DPClient, created_user_ids: List[str], require_db: None) -> None: + payload: Dict[str, object] = { "id": generate_israeli_id(), "name": "Test User", "phone": generate_random_phone_number(), "address": "Test Street 1", } - created_user_ids.append(payload["id"]) + created_user_ids.append(str(payload["id"])) create_resp = client.create_user(payload) assert create_resp.status_code == HTTPStatus.CREATED - # Verify in DB first via dp-client - assert client.PGDBClient.users_exist([payload["id"]]) + assert client.PGDBClient.users_exist([str(payload["id"])]) - response = client.get_user(payload["id"]) + response = client.get_user(str(payload["id"])) assert response.status_code == HTTPStatus.OK + assert response.parsed is not None assert response.parsed.id == payload["id"] assert response.parsed.name == payload["name"] - # Cleanup: delete and verify DB removal - del_resp = client.delete_user(payload["id"]) # 204 expected + del_resp = client.delete_user(str(payload["id"])) # 204 expected assert del_resp.status_code == HTTPStatus.NO_CONTENT - assert client.PGDBClient.get_user_by_id(payload["id"]) is None + assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None -def test_list_users_component(client: DPClient, created_user_ids, require_db): - users_data = [ +def test_list_users_component(client: DPClient, created_user_ids: List[str], require_db: None) -> None: + users_data: List[Dict[str, object]] = [ { "id": generate_israeli_id(), "name": "Test User", @@ -176,32 +171,27 @@ def test_list_users_component(client: DPClient, created_user_ids, require_db): }, ] for u in users_data: - created_user_ids.append(u["id"]) + created_user_ids.append(str(u["id"])) resp = client.create_user(u) assert resp.status_code == HTTPStatus.CREATED - # DB cross-check: all created users present (via dp-client DB driver) - ids = [u["id"] for u in users_data] + ids = [str(u["id"]) for u in users_data] assert client.PGDBClient.users_exist(ids) - # API list still OK list_resp = client.list_users() assert list_resp.status_code == HTTPStatus.OK + assert list_resp.parsed is not None returned_ids_api = {u.id for u in list_resp.parsed} for u in users_data: assert u["id"] in returned_ids_api - # Cleanup: delete all created users via API and validate DB removal for u in users_data: - del_resp = client.delete_user(u["id"]) # 204 expected + del_resp = client.delete_user(str(u["id"])) # 204 expected assert del_resp.status_code == HTTPStatus.NO_CONTENT - assert client.PGDBClient.get_user_by_id(u["id"]) is None - + assert client.PGDBClient.get_user_by_id(str(u["id"])) is None -# ---- Parametrized update/patch and delete with DB validations ---- - -def _new_user_payload() -> dict: +def _new_user_payload() -> dict[str, object]: return { "id": generate_israeli_id(), "name": "Comp A", @@ -221,24 +211,31 @@ def _new_user_payload() -> dict: @pytest.mark.usefixtures("require_db") @pytest.mark.parametrize("payload_builder,expect_ok", PATCH_CASES_COMPONENT) -def test_component_users_partial_update_parametrized(client: DPClient, created_user_ids, payload_builder, expect_ok): +def test_component_users_partial_update_parametrized( + client: DPClient, + created_user_ids: List[str], + payload_builder: Callable[[], Dict[str, object]], + expect_ok: bool, +) -> None: payload = _new_user_payload() - created_user_ids.append(payload["id"]) # ensure cleanup + created_user_ids.append(str(payload["id"])) # ensure cleanup create_resp = client.create_user(payload) assert create_resp.status_code == HTTPStatus.CREATED patch_body = payload_builder() - resp = client.partial_update_user(payload["id"], patch_body) + resp = client.partial_update_user(str(payload["id"]), patch_body) if expect_ok: assert resp.status_code == HTTPStatus.OK + assert resp.parsed is not None for k, v in patch_body.items(): if k == "id": continue assert getattr(resp.parsed, k) == v # DB verify - db = client.PGDBClient.get_user_by_id(payload["id"]) + db = client.PGDBClient.get_user_by_id(str(payload["id"])) + assert db is not None for k, v in patch_body.items(): if k != "id": assert db[k] == v @@ -246,13 +243,13 @@ def test_component_users_partial_update_parametrized(client: DPClient, created_u assert resp.status_code == HTTPStatus.BAD_REQUEST assert resp.parsed is None # DB should remain unchanged - db = client.PGDBClient.get_user_by_id(payload["id"]) + db = client.PGDBClient.get_user_by_id(str(payload["id"])) assert db is not None # Cleanup: delete via API and validate DB removal - del_resp = client.delete_user(payload["id"]) # 204 expected + del_resp = client.delete_user(str(payload["id"])) # 204 expected assert del_resp.status_code == HTTPStatus.NO_CONTENT - assert client.PGDBClient.get_user_by_id(payload["id"]) is None + assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None PUT_CASES_COMPONENT = [ @@ -281,25 +278,31 @@ def test_component_users_partial_update_parametrized(client: DPClient, created_u @pytest.mark.usefixtures("require_db") @pytest.mark.parametrize("payload_builder,expect_ok", PUT_CASES_COMPONENT) -def test_component_users_update_put_parametrized(client: DPClient, created_user_ids, payload_builder, expect_ok): +def test_component_users_update_put_parametrized( + client: DPClient, + created_user_ids: List[str], + payload_builder: Callable[[], Dict[str, object]], + expect_ok: bool, +) -> None: payload = _new_user_payload() - created_user_ids.append(payload["id"]) # ensure cleanup + created_user_ids.append(str(payload["id"])) # ensure cleanup create_resp = client.create_user(payload) assert create_resp.status_code == HTTPStatus.CREATED put_body = payload_builder() - resp = client.update_user(payload["id"], put_body) + resp = client.update_user(str(payload["id"]), put_body) if expect_ok: assert resp.status_code == HTTPStatus.OK + assert resp.parsed is not None assert resp.parsed.id == payload["id"] for k, v in put_body.items(): if k == "id": continue assert getattr(resp.parsed, k) == v - # DB verify - db = client.PGDBClient.get_user_by_id(payload["id"]) + db = client.PGDBClient.get_user_by_id(str(payload["id"])) + assert db is not None for k, v in put_body.items(): if k != "id": assert db[k] == v @@ -307,7 +310,6 @@ def test_component_users_update_put_parametrized(client: DPClient, created_user_ assert resp.status_code == HTTPStatus.BAD_REQUEST assert resp.parsed is None - # Cleanup: delete via API and validate DB removal - del_resp = client.delete_user(payload["id"]) # 204 expected + del_resp = client.delete_user(str(payload["id"])) # 204 expected assert del_resp.status_code == HTTPStatus.NO_CONTENT - assert client.PGDBClient.get_user_by_id(payload["id"]) is None + assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None diff --git a/tests/conftest.py b/tests/conftest.py index d7b27eb..6bb70da 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,7 +3,7 @@ import os import sys from pathlib import Path -from typing import Any +from typing import Any, cast import pytest import requests @@ -70,7 +70,8 @@ def auth_token(base_url: str) -> str: timeout=10, ) resp.raise_for_status() - return resp.json()["access"] + data = cast(dict[str, Any], resp.json()) + return cast(str, data["access"]) try: @@ -82,7 +83,7 @@ def auth_token(base_url: str) -> str: @pytest.fixture(scope="session", autouse=True) -def non_authenticated_client(base_url: str): +def non_authenticated_client(base_url: str) -> _Client: """Unauthenticated generated API client (metadata_client.Client). Args: @@ -91,13 +92,11 @@ def non_authenticated_client(base_url: str): Returns: metadata_client.Client instance. """ - if Client is None: - raise RuntimeError("metadata_client is not installed. Generate or install the client before running tests.") - return Client(base_url=base_url) + return _Client(base_url=base_url) @pytest.fixture(scope="session", autouse=True) -def client(base_url: str, auth_token: str): +def client(base_url: str, auth_token: str) -> _AuthenticatedClient: """Authenticated generated API client (metadata_client.AuthenticatedClient). Args: @@ -107,9 +106,7 @@ def client(base_url: str, auth_token: str): Returns: metadata_client.AuthenticatedClient instance. """ - if AuthenticatedClient is None: - raise RuntimeError("metadata_client is not installed. Generate or install the client before running tests.") - return AuthenticatedClient(base_url=base_url, token=auth_token, prefix="Bearer") + return _AuthenticatedClient(base_url=base_url, token=auth_token, prefix="Bearer") @pytest.fixture(scope="function") diff --git a/tests/unit/test_user_model.py b/tests/unit/test_user_model.py index 9518596..d1766f4 100644 --- a/tests/unit/test_user_model.py +++ b/tests/unit/test_user_model.py @@ -1,4 +1,5 @@ from http import HTTPStatus +from typing import Any, Callable import pytest from metadata_client import AuthenticatedClient @@ -15,7 +16,7 @@ from src.tools import generate_israeli_id, generate_random_phone_number -def test_health_check(non_authenticated_client): +def test_health_check(non_authenticated_client: Any) -> None: response = health_retrieve.sync_detailed(client=non_authenticated_client) assert response.status_code == HTTPStatus.OK @@ -40,13 +41,14 @@ def test_health_check(non_authenticated_client): @pytest.mark.parametrize("user_builder,expect_ok", CREATE_CASES) -def test_users_create_parametrized(client, user_builder, expect_ok): +def test_users_create_parametrized(client: Any, user_builder: Callable[[], User], expect_ok: bool) -> None: user = user_builder() resp = users_create.sync_detailed(client=client, body=user) if expect_ok: assert resp.status_code == HTTPStatus.CREATED + assert resp.parsed is not None assert resp.parsed.id == user.id - # Cleanup: delete via API and validate removal + del_resp = users_destroy.sync_detailed(client=client, id=user.id) assert del_resp.status_code == HTTPStatus.NO_CONTENT get_resp = users_retrieve.sync_detailed(client=client, id=user.id) @@ -66,7 +68,12 @@ def test_users_create_parametrized(client, user_builder, expect_ok): @pytest.mark.parametrize("payload_builder,expect_ok", PATCH_CASES) -def test_users_partial_update_parametrized(client: AuthenticatedClient, payload_builder, expect_ok, new_user: User): +def test_users_partial_update_parametrized( + client: AuthenticatedClient, + payload_builder: Callable[[], dict[str, Any]], + expect_ok: bool, + new_user: User, +) -> None: user = new_user create_resp = users_create.sync_detailed(client=client, body=user) assert create_resp.status_code == HTTPStatus.CREATED @@ -87,7 +94,6 @@ def test_users_partial_update_parametrized(client: AuthenticatedClient, payload_ assert resp.status_code == HTTPStatus.BAD_REQUEST assert resp.parsed is None - # Cleanup: delete the created user and validate deletion del_resp = users_destroy.sync_detailed(client=client, id=user.id) assert del_resp.status_code == HTTPStatus.NO_CONTENT get_resp = users_retrieve.sync_detailed(client=client, id=user.id) @@ -96,13 +102,11 @@ def test_users_partial_update_parametrized(client: AuthenticatedClient, payload_ PUT_CASES = [ pytest.param( - # OK: client sends id equal to path (many generators require id in model) lambda cur_id: {"id": cur_id, "name": "User B", "phone": generate_random_phone_number(), "address": "Addr B"}, True, id="put-with-same-id:ok", ), pytest.param( - # Forbidden: client sends a different id attempting to change PK lambda cur_id: { "id": generate_israeli_id(), "name": "User B", @@ -113,7 +117,6 @@ def test_users_partial_update_parametrized(client: AuthenticatedClient, payload_ id="put-with-different-id:forbidden", ), pytest.param( - # Invalid phone while keeping same id lambda cur_id: {"id": cur_id, "name": "User C", "phone": "0501234567", "address": "Addr C"}, False, id="put-invalid-phone:bad-format", @@ -122,7 +125,12 @@ def test_users_partial_update_parametrized(client: AuthenticatedClient, payload_ @pytest.mark.parametrize("payload_builder,expect_ok", PUT_CASES) -def test_users_update_put_parametrized(client: AuthenticatedClient, payload_builder, expect_ok, new_user: User): +def test_users_update_put_parametrized( + client: AuthenticatedClient, + payload_builder: Any, + expect_ok: bool, + new_user: User, +) -> None: user = new_user create_resp = users_create.sync_detailed(client=client, body=user) assert create_resp.status_code == HTTPStatus.CREATED @@ -143,7 +151,6 @@ def test_users_update_put_parametrized(client: AuthenticatedClient, payload_buil assert resp.status_code == HTTPStatus.BAD_REQUEST assert resp.parsed is None - # Cleanup: delete the created user and validate deletion del_resp = users_destroy.sync_detailed(client=client, id=user.id) assert del_resp.status_code == HTTPStatus.NO_CONTENT get_resp = users_retrieve.sync_detailed(client=client, id=user.id) From c4efe45715c39663b5c388028fafd6d1e628ec7b Mon Sep 17 00:00:00 2001 From: sidkos Date: Sun, 5 Oct 2025 18:06:49 +0300 Subject: [PATCH 4/7] wip4 --- dp-client/dp_client/api/users.py | 4 ++-- dp-client/dp_client/client.py | 2 -- dp-client/dp_client/db/drivers/postgres.py | 2 -- tests/component/test_user_model_component.py | 17 +++++++---------- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/dp-client/dp_client/api/users.py b/dp-client/dp_client/api/users.py index 4f3823a..c1c13f8 100644 --- a/dp-client/dp_client/api/users.py +++ b/dp-client/dp_client/api/users.py @@ -10,6 +10,8 @@ from metadata_client.models import User as _User from metadata_client.models import UserUpdate as _UserUpdate +from typing import cast + class UsersAPI: """Users endpoints wrapper using generated metadata_client api module. @@ -76,8 +78,6 @@ def __init__(self, client: Union[Client, AuthenticatedClient]) -> None: + ", ".join(missing) + ". Regenerate the client from the updated API spec." ) - # Cast to non-optional after validation - from typing import cast self._ep: Dict[str, ModuleType] = cast(Dict[str, ModuleType], tmp_ep) diff --git a/dp-client/dp_client/client.py b/dp-client/dp_client/client.py index a0bc912..2075092 100644 --- a/dp-client/dp_client/client.py +++ b/dp-client/dp_client/client.py @@ -88,7 +88,6 @@ def list_users(self) -> Response[list[_User]]: """ return self.UsersApi.list_users() - # New endpoints wrappers def update_user(self, user_id: str, body: Union[_UserUpdate, dict[str, object]]) -> Response[_User]: """Update a user via PUT /api/users/{id}/. @@ -99,7 +98,6 @@ def update_user(self, user_id: str, body: Union[_UserUpdate, dict[str, object]]) Returns: The detailed response from the generated client. """ - # Delegate to UsersAPI which normalizes payload and types return self.UsersApi.update_user(user_id, body) def partial_update_user(self, user_id: str, body: dict[str, object]) -> Response[_User]: diff --git a/dp-client/dp_client/db/drivers/postgres.py b/dp-client/dp_client/db/drivers/postgres.py index 18c141b..f38b73d 100644 --- a/dp-client/dp_client/db/drivers/postgres.py +++ b/dp-client/dp_client/db/drivers/postgres.py @@ -46,14 +46,12 @@ def fetch_value(self, query: str, params: Optional[Tuple[Any, ...]] = None) -> O try: return int(row[0]) except (TypeError, ValueError): - # Fallback: not an int-compatible value return None def execute(self, query: str, params: Optional[Tuple[Any, ...]] = None) -> int: with self._conn() as conn: with conn.cursor() as cur: cur.execute(query, params or ()) - # psycopg2 rowcount: number of rows affected by last command affected = cur.rowcount conn.commit() return int(affected) diff --git a/tests/component/test_user_model_component.py b/tests/component/test_user_model_component.py index 4b4dfa5..1e61d70 100644 --- a/tests/component/test_user_model_component.py +++ b/tests/component/test_user_model_component.py @@ -97,7 +97,7 @@ def test_create_user_valid_component(client: DPClient, created_user_ids: List[st assert db_user["phone"] == payload["phone"] assert db_user["address"] == payload["address"] - del_resp = client.delete_user(str(payload["id"])) # 204 expected + del_resp = client.delete_user(str(payload["id"])) assert del_resp.status_code == HTTPStatus.NO_CONTENT assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None @@ -150,7 +150,7 @@ def test_retrieve_user_component(client: DPClient, created_user_ids: List[str], assert response.parsed.id == payload["id"] assert response.parsed.name == payload["name"] - del_resp = client.delete_user(str(payload["id"])) # 204 expected + del_resp = client.delete_user(str(payload["id"])) assert del_resp.status_code == HTTPStatus.NO_CONTENT assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None @@ -186,7 +186,7 @@ def test_list_users_component(client: DPClient, created_user_ids: List[str], req assert u["id"] in returned_ids_api for u in users_data: - del_resp = client.delete_user(str(u["id"])) # 204 expected + del_resp = client.delete_user(str(u["id"])) assert del_resp.status_code == HTTPStatus.NO_CONTENT assert client.PGDBClient.get_user_by_id(str(u["id"])) is None @@ -218,7 +218,7 @@ def test_component_users_partial_update_parametrized( expect_ok: bool, ) -> None: payload = _new_user_payload() - created_user_ids.append(str(payload["id"])) # ensure cleanup + created_user_ids.append(str(payload["id"])) create_resp = client.create_user(payload) assert create_resp.status_code == HTTPStatus.CREATED @@ -233,7 +233,6 @@ def test_component_users_partial_update_parametrized( if k == "id": continue assert getattr(resp.parsed, k) == v - # DB verify db = client.PGDBClient.get_user_by_id(str(payload["id"])) assert db is not None for k, v in patch_body.items(): @@ -242,12 +241,10 @@ def test_component_users_partial_update_parametrized( else: assert resp.status_code == HTTPStatus.BAD_REQUEST assert resp.parsed is None - # DB should remain unchanged db = client.PGDBClient.get_user_by_id(str(payload["id"])) assert db is not None - # Cleanup: delete via API and validate DB removal - del_resp = client.delete_user(str(payload["id"])) # 204 expected + del_resp = client.delete_user(str(payload["id"])) assert del_resp.status_code == HTTPStatus.NO_CONTENT assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None @@ -285,7 +282,7 @@ def test_component_users_update_put_parametrized( expect_ok: bool, ) -> None: payload = _new_user_payload() - created_user_ids.append(str(payload["id"])) # ensure cleanup + created_user_ids.append(str(payload["id"])) create_resp = client.create_user(payload) assert create_resp.status_code == HTTPStatus.CREATED @@ -310,6 +307,6 @@ def test_component_users_update_put_parametrized( assert resp.status_code == HTTPStatus.BAD_REQUEST assert resp.parsed is None - del_resp = client.delete_user(str(payload["id"])) # 204 expected + del_resp = client.delete_user(str(payload["id"])) assert del_resp.status_code == HTTPStatus.NO_CONTENT assert client.PGDBClient.get_user_by_id(str(payload["id"])) is None From 91785be97dd0cf5836b613ff9058214b3ac79b2c Mon Sep 17 00:00:00 2001 From: sidkos Date: Sun, 5 Oct 2025 18:15:40 +0300 Subject: [PATCH 5/7] wip5 --- tests/component/conftest.py | 102 ++++++++++++++++++ tests/component/test_user_model_component.py | 106 +++++++------------ 2 files changed, 143 insertions(+), 65 deletions(-) create mode 100644 tests/component/conftest.py diff --git a/tests/component/conftest.py b/tests/component/conftest.py new file mode 100644 index 0000000..ce060c9 --- /dev/null +++ b/tests/component/conftest.py @@ -0,0 +1,102 @@ +from __future__ import annotations + +from typing import Generator, List + +import pytest +from dp_client import DPClient + +# psycopg2-related exception tuple to allow conditional DB cleanup handling +PSYCOPG2_ERRORS: tuple[type[BaseException], ...] = tuple() +try: + import psycopg2 + + PSYCOPG2_ERRORS = ( + psycopg2.OperationalError, + psycopg2.InterfaceError, + psycopg2.Error, + ) +except Exception: + # psycopg2 not installed or otherwise unavailable in this environment + pass + + +@pytest.fixture +def non_authenticated_client(base_url: str) -> DPClient: + """Build a DPClient without authentication for public endpoints. + + Args: + base_url: Base URL for the API under test. + + Returns: + DPClient: Client configured without Authorization header. + """ + return DPClient(base_url=base_url) + + +@pytest.fixture +def client(base_url: str, auth_token: str) -> DPClient: + """Build an authenticated DPClient for protected endpoints. + + Args: + base_url: Base URL for the API under test. + auth_token: Bearer token to use for authenticated requests. + + Returns: + DPClient: Client configured with Authorization header. + """ + return DPClient(base_url=base_url, token=auth_token, prefix="Bearer") + + +@pytest.fixture +def require_db(client: DPClient) -> None: + """Skip tests requiring DB access if the DB is not reachable. + + Tries a simple query via PGDBClient; on failure, skips the test. + + Args: + client: Authenticated DPClient instance. + + Returns: + None. This is a guard fixture that may skip the test. + """ + try: + # Trigger a DB connection with a harmless lookup + client.PGDBClient.get_user_by_id("__nonexistent__") + except Exception as exc: + pytest.skip( + "Database not available for component DB checks: " + f"{exc}\n" + "Hints: set POSTGRES_HOST to a reachable host (e.g., localhost), " + "or export POSTGRES_ALLOW_LOCAL_FALLBACK=true to fall back to localhost when 'db' is not resolvable, " + "or run `docker-compose up -d` so the service name 'db' is resolvable." + ) + + +@pytest.fixture(scope="function") +def created_user_ids(client: DPClient) -> Generator[List[str], None, None]: + """Track user IDs created during a test and ensure cleanup. + + Args: + client: Authenticated DPClient instance. + + Yields: + A mutable list to which tests append created IDs for automatic cleanup. + """ + ids: List[str] = [] + # Defensive pre-cleanup if list is pre-populated for any reason + if ids: + try: + client.PGDBClient.delete_users_by_ids(ids) + except PSYCOPG2_ERRORS + (RuntimeError,): + # DB not available/misconfigured; ignore cleanup in pre + pass + + try: + yield ids + finally: + if ids: + try: + client.PGDBClient.delete_users_by_ids(ids) + except PSYCOPG2_ERRORS + (RuntimeError,): + # DB not available/misconfigured; ignore cleanup in post + pass diff --git a/tests/component/test_user_model_component.py b/tests/component/test_user_model_component.py index 1e61d70..4648af5 100644 --- a/tests/component/test_user_model_component.py +++ b/tests/component/test_user_model_component.py @@ -1,82 +1,28 @@ from http import HTTPStatus -from typing import Callable, Dict, Generator, List +from typing import Callable, Dict, List import pytest from dp_client import DPClient from src.tools import generate_israeli_id, generate_random_phone_number -# Define a tuple of psycopg2-related exceptions if psycopg2 is available. -# This lets us catch specific DB connectivity/driver errors without a blanket Exception. -PSYCOPG2_ERRORS: tuple[type[BaseException], ...] = tuple() -try: - import psycopg2 - - PSYCOPG2_ERRORS = ( - psycopg2.OperationalError, - psycopg2.InterfaceError, - psycopg2.Error, - ) -except Exception: # psycopg2 not installed or failed to import in this context - # Leave PSYCOPG2_ERRORS as an empty tuple of exception types - pass - - -@pytest.fixture -def non_authenticated_client(base_url: str) -> DPClient: - return DPClient(base_url=base_url) - - -@pytest.fixture -def client(base_url: str, auth_token: str) -> DPClient: - return DPClient(base_url=base_url, token=auth_token, prefix="Bearer") - - -@pytest.fixture -def require_db(client: DPClient) -> None: - """Skip tests that require direct DB access if the DB is not reachable. - - Tries a simple query via PGDBClient; on failure, skips the test. - """ - try: - # Trigger a DB connection with a harmless lookup - client.PGDBClient.get_user_by_id("__nonexistent__") - except Exception as exc: - pytest.skip( - "Database not available for component DB checks: " - f"{exc}\n" - "Hints: set POSTGRES_HOST to a reachable host (e.g., localhost), " - "or export POSTGRES_ALLOW_LOCAL_FALLBACK=true to fall back to localhost when 'db' is not resolvable, " - "or run `docker-compose up -d` so the service name 'db' is resolvable." - ) - - -@pytest.fixture(scope="function") -def created_user_ids(client: DPClient) -> Generator[List[str], None, None]: - ids: List[str] = [] - # Pre-test cleanup (defensive): ensure none of these ids already exist (normally random, but be safe) - if ids: - try: - client.PGDBClient.delete_users_by_ids(ids) - except PSYCOPG2_ERRORS + (RuntimeError,): - # DB not available/misconfigured; ignore cleanup in pre - pass - yield ids - # Post-test cleanup: remove created users directly from DB since there is no DELETE API - if ids: - try: - client.PGDBClient.delete_users_by_ids(ids) - except PSYCOPG2_ERRORS + (RuntimeError,): - # DB not available/misconfigured; ignore cleanup in post - pass - def test_health_check_component(non_authenticated_client: DPClient) -> None: + """Test health endpoint responds with 200 OK. + 1. Build non-authenticated client + 2. Call GET /api/health/ + 3. Assert HTTP 200 + """ response = non_authenticated_client.health_check() assert response.status_code == HTTPStatus.OK def test_create_user_valid_component(client: DPClient, created_user_ids: List[str], require_db: None) -> None: + """Test creating a valid user and DB persistence + cleanup. + 1. Create a user via API + 2. Verify response body and DB state + 3. Delete user via API and verify DB removal + """ payload: Dict[str, object] = { "id": generate_israeli_id(), "name": "Test User", @@ -103,6 +49,11 @@ def test_create_user_valid_component(client: DPClient, created_user_ids: List[st def test_create_user_invalid_id_component(client: DPClient, require_db: None) -> None: + """Test creating a user with invalid ID fails and no DB insert occurs. + 1. Build invalid payload with bad ID + 2. Call POST /api/users/ + 3. Assert 400 and no DB record exists + """ payload: Dict[str, object] = { "id": "123789456", "name": "Test User", @@ -117,6 +68,11 @@ def test_create_user_invalid_id_component(client: DPClient, require_db: None) -> def test_create_user_invalid_phone_component(client: DPClient, require_db: None) -> None: + """Test creating a user with invalid phone fails and no DB insert occurs. + 1. Build invalid payload with bad phone + 2. Call POST /api/users/ + 3. Assert 400 and no DB record exists + """ payload: Dict[str, object] = { "id": generate_israeli_id(), "name": "Test User", @@ -131,6 +87,11 @@ def test_create_user_invalid_phone_component(client: DPClient, require_db: None) def test_retrieve_user_component(client: DPClient, created_user_ids: List[str], require_db: None) -> None: + """Test retrieving a user reflects created data and cleanup works. + 1. Create a user via API + 2. Retrieve user via API and verify fields + 3. Delete user and verify DB cleanup + """ payload: Dict[str, object] = { "id": generate_israeli_id(), "name": "Test User", @@ -156,6 +117,11 @@ def test_retrieve_user_component(client: DPClient, created_user_ids: List[str], def test_list_users_component(client: DPClient, created_user_ids: List[str], require_db: None) -> None: + """Test listing users contains created users and cleanup works. + 1. Create two users via API + 2. List users and verify IDs included + 3. Delete users and verify DB cleanup + """ users_data: List[Dict[str, object]] = [ { "id": generate_israeli_id(), @@ -217,6 +183,11 @@ def test_component_users_partial_update_parametrized( payload_builder: Callable[[], Dict[str, object]], expect_ok: bool, ) -> None: + """Test PATCH variants (parametrized) and DB validation + cleanup. + 1. Create user via API + 2. PATCH with parametrized body and assert success or failure + 3. Verify DB state accordingly and delete user + """ payload = _new_user_payload() created_user_ids.append(str(payload["id"])) @@ -281,6 +252,11 @@ def test_component_users_update_put_parametrized( payload_builder: Callable[[], Dict[str, object]], expect_ok: bool, ) -> None: + """Test PUT variants (parametrized) with strict id and DB validation. + 1. Create user via API + 2. PUT with parametrized body and assert success or failure + 3. Verify DB state accordingly and delete user + """ payload = _new_user_payload() created_user_ids.append(str(payload["id"])) From f6d7231c499a7582a930c0fb5fcca175a495d8a5 Mon Sep 17 00:00:00 2001 From: sidkos Date: Sun, 5 Oct 2025 18:18:46 +0300 Subject: [PATCH 6/7] wip6 --- README.md | 7 +++++-- dp-client/README.md | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 87799ae..e9d0ea0 100644 --- a/README.md +++ b/README.md @@ -33,11 +33,14 @@ When to use what: # API surface (summary) - POST /api/users/ — Create a user - GET /api/users/{id}/ — Retrieve user by ID -- GET /api/users/ — List all user IDs +- GET /api/users/ — List users +- PUT /api/users/{id}/ — Update a user (id is immutable; if provided in body it must equal the path id) +- PATCH /api/users/{id}/ — Partially update a user (id in body is forbidden) +- DELETE /api/users/{id}/ — Delete a user - GET /api/health/ — Public health check (no auth) Validation rules: -- id: Valid Israeli ID (checksum validated) +- id: Valid Israeli ID (checksum validated). Primary key is immutable after creation. - phone: International format starting with `+` - name: Required - address: Required diff --git a/dp-client/README.md b/dp-client/README.md index 4a67b36..80dda73 100644 --- a/dp-client/README.md +++ b/dp-client/README.md @@ -13,7 +13,7 @@ A higher-level, test-friendly Python client for the Metadata Server, designed fo `DPClient` composes distinct parts with strict separation of concerns: - MetaDataServerAPIClient — factory‑built HTTP client for the Metadata Server (authenticated or not). - HealthAPI — methods for `/api/health/`. -- UsersApi — methods for `/api/users/` (create/get/list), aligned with unit tests usage. +- UsersApi — methods for `/api/users/` (create/get/list/update/partial_update/destroy) with strict id immutability enforced by the server. The client injects the path id into PUT bodies if omitted, and rejects PATCH bodies containing `id`. - PGDBClient — optional Postgres DB helper used in tests to verify persistence and cleanup. It is decoupled from Django ORM and uses driver(s) under `dp_client.db.drivers`. This design keeps HTTP API usage and DB verification separate, while allowing simple orchestration from tests. @@ -93,10 +93,25 @@ if client.PGDBClient is not None: Backwards‑compatible helpers (delegating to structured APIs): ```python +from dp_client import DPClient +client = DPClient(base_url="http://localhost:8000", token="") + +# Health client.health_check() + +# Users CRUD client.create_user({"id": "...", "name": "...", "phone": "+972...", "address": "..."}) -client.get_user("...") +client.get_user("") client.list_users() + +# Update (PUT): body must represent the same id as in the path; if omitted, dp-client injects the path id +client.update_user("", {"name": "New", "phone": "+9728...", "address": "..."}) + +# Partial update (PATCH): must NOT include 'id' in body +client.partial_update_user("", {"address": "Changed"}) + +# Delete +client.delete_user("") ``` ## Using dp-client in component tests From 81152d4b2825f3f8e1934f9791362cdcf88fc1c4 Mon Sep 17 00:00:00 2001 From: sidkos Date: Sun, 5 Oct 2025 18:21:16 +0300 Subject: [PATCH 7/7] wip 7 --- README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/README.md b/README.md index e9d0ea0..503a649 100644 --- a/README.md +++ b/README.md @@ -192,3 +192,30 @@ Pipeline summary: - Introduce semantic versioning for all deliverables. - Publish metadata_client and dp-client to a registry and pin versions in tests and downstream projects. - Keep dp-client in a separate repository with its own SDLC, releases, and changelog. + + +# Test matrix + +The repository contains unit tests (using the generated metadata_client) and component tests (using the higher-level dp-client with optional DB validations). Below is an overview of the current tests and what they verify. + +| Suite | Test function | Parametrized case IDs | What it verifies | DB checks | +|------------|---------------------------------------------------|------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------|-----------| +| Unit | test_health_check | — | GET /api/health/ returns 200 OK | No | +| Unit | test_users_create_parametrized | create:ok, create:bad-id, create:bad-phone | POST /api/users/ happy path and validation failures | No | +| Unit | test_users_partial_update_parametrized | patch-address:ok, patch-name:ok, patch-phone:ok-valid, patch-phone:bad-format, patch-id:forbidden | PATCH /api/users/{id}/; validators apply; id in body forbidden | No | +| Unit | test_users_update_put_parametrized | put-with-same-id:ok, put-with-different-id:forbidden, put-invalid-phone:bad-format | PUT /api/users/{id}/; id immutable (must match path); validators apply | No | +| Component | test_health_check_component | — | GET /api/health/ returns 200 OK | No | +| Component | test_create_user_valid_component | — | Create user via API and verify response; then verify row exists and delete it | Yes | +| Component | test_create_user_invalid_id_component | — | Invalid ID rejected by API; ensure no row inserted | Yes | +| Component | test_create_user_invalid_phone_component | — | Invalid phone rejected by API; ensure no row inserted | Yes | +| Component | test_retrieve_user_component | — | Create and then GET /api/users/{id}/ reflects fields; cleanup and DB removal | Yes | +| Component | test_list_users_component | — | Create two users, list includes their IDs; cleanup and DB removal | Yes | +| Component | test_component_users_partial_update_parametrized | component:patch-address:ok, component:patch-name:ok, component:patch-phone:ok, component:patch-phone:bad-format, component:patch-id:forbidden | PATCH /api/users/{id}/ happy and failure paths; DB reflects successful changes; id in body forbidden | Yes | +| Component | test_component_users_update_put_parametrized | component:put-no-id:ok, component:put-with-id:forbidden, component:put-invalid-phone:bad-format | PUT /api/users/{id}/ happy and failure paths; server enforces immutable id; DB reflects successful changes | Yes | + +How to run tests: +- Unit tests: `pytest tests/unit -s -v` +- Component tests (with DB validations): + - Optionally enable fallback if running outside Docker and `.env` uses POSTGRES_HOST=db: `export POSTGRES_ALLOW_LOCAL_FALLBACK=true` + - Build and install dp-client: `./scripts/build_and_install_dp_client.sh` + - Run: `pytest tests/component -s -v`