From d13d52e1b835a0d032e0a5b0301fa0d12579331c Mon Sep 17 00:00:00 2001 From: Sivaselvan32 Date: Fri, 29 Aug 2025 11:18:48 +0530 Subject: [PATCH 1/6] Added Abstract Endpoint --- tests/units/test_endpoint.py | 304 +++++++++++++++++++++++++++++++++++ tfe/__init__.py | 3 +- tfe/endpoint.py | 170 ++++++++++++++++++++ 3 files changed, 476 insertions(+), 1 deletion(-) create mode 100644 tests/units/test_endpoint.py create mode 100644 tfe/endpoint.py diff --git a/tests/units/test_endpoint.py b/tests/units/test_endpoint.py new file mode 100644 index 00000000..b910bbca --- /dev/null +++ b/tests/units/test_endpoint.py @@ -0,0 +1,304 @@ +"""Unit tests for the Endpoint class.""" + +from typing import Any +from unittest.mock import Mock + +import pytest + +from tfe.endpoint import Endpoint, ResourceDataProtocol, ResourceResponseProtocol + + +class MockResourceData: + """Mock resource data for testing.""" + + def __init__(self, id: str, type: str, **attributes: Any): + self.id = id + self.type = type + self.attributes = attributes + + +class MockResourceResponse: + """Mock resource response for testing.""" + + def __init__(self, data: ResourceDataProtocol | list[ResourceDataProtocol]): + self.data = data + + +# Mock service implementation for testing +class MockService(Endpoint): + """Mock service implementation for testing Endpoint.""" + + def list_resources( + self, + page: int | None = None, + per_page: int | None = None, + search: str | None = None, + include: list[str] | None = None, + filter: dict[str, str] | None = None, + sort: str | None = None, + **additional_params: Any, + ) -> ResourceResponseProtocol: + """List resources.""" + # Build parameters dict + params = {} + if page is not None: + params["page"] = page + if per_page is not None: + params["per_page"] = per_page + if search is not None: + params["search"] = search + if include is not None: + params["include"] = include + if filter is not None: + params["filter"] = filter + if sort is not None: + params["sort"] = sort + # Add additional params + params.update(additional_params) + + self._make_request("GET", "mock-resources", **params) + mock_data = MockResourceData(id="1", type="mock", name="test") + return MockResourceResponse(data=[mock_data]) + + def get_resource( + self, + resource_id: str, + include: list[str] | None = None, + **additional_params: Any, + ) -> ResourceResponseProtocol: + """Get a single resource.""" + self._make_request("GET", f"mock-resources/{resource_id}", **additional_params) + mock_data = MockResourceData(id=resource_id, type="mock", name="test") + return MockResourceResponse(data=mock_data) + + def create_resource( + self, data: dict[str, Any], **additional_params: Any + ) -> ResourceResponseProtocol: + """Create a resource.""" + self._make_request("POST", "mock-resources", json=data) + mock_data = MockResourceData(id="new", type="mock", **data) + return MockResourceResponse(data=mock_data) + + def update_resource( + self, resource_id: str, data: dict[str, Any], **additional_params: Any + ) -> ResourceResponseProtocol: + """Update a resource.""" + self._make_request("PATCH", f"mock-resources/{resource_id}", json=data) + mock_data = MockResourceData(id=resource_id, type="mock", **data) + return MockResourceResponse(data=mock_data) + + def delete_resource(self, resource_id: str, **additional_params: Any) -> bool: + """Delete a resource.""" + response = self._make_request("DELETE", f"mock-resources/{resource_id}") + return response.status_code == 204 + + +class TestEndpoint: + """Test the Endpoint class.""" + + @pytest.fixture + def mock_client(self): + """Create a mock client with standard configuration.""" + mock_client = Mock() + mock_client.config = Mock() + mock_client.config.http_client = Mock() + mock_client.base_url = "https://api.example.com" + return mock_client + + @pytest.fixture + def mock_http_client(self): + """Create a mock HTTP client.""" + return Mock() + + @pytest.fixture + def mock_response(self): + """Create a mock response.""" + return Mock() + + def _setup_endpoint(self, mock_client, mock_http_client, mock_response): + """Helper method to setup endpoint with mocked dependencies.""" + mock_http_client.get.return_value = mock_response + mock_http_client.post.return_value = mock_response + mock_http_client.put.return_value = mock_response + mock_http_client.patch.return_value = mock_response + mock_http_client.delete.return_value = mock_response + + mock_client.config.http_client = mock_http_client + return MockService(mock_client) + + def test_init(self, mock_client): + """Test endpoint initialization.""" + endpoint = MockService(mock_client) + + assert endpoint._http_client == mock_client.config.http_client + assert endpoint._base_url == "https://api.example.com" + + def test_make_request_get(self, mock_client, mock_http_client, mock_response): + """Test making a GET request.""" + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint._make_request("GET", "test-endpoint") + + mock_http_client.get.assert_called_once_with( + "https://api.example.com/test-endpoint" + ) + assert result == mock_response + + def test_make_request_post(self, mock_client, mock_http_client, mock_response): + """Test making a POST request.""" + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint._make_request("POST", "test-endpoint", json={"key": "value"}) + + mock_http_client.post.assert_called_once_with( + "https://api.example.com/test-endpoint", json={"key": "value"} + ) + assert result == mock_response + + def test_make_request_put(self, mock_client, mock_http_client, mock_response): + """Test making a PUT request.""" + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint._make_request("PUT", "test-endpoint", json={"key": "value"}) + + mock_http_client.put.assert_called_once_with( + "https://api.example.com/test-endpoint", json={"key": "value"} + ) + assert result == mock_response + + def test_make_request_patch(self, mock_client, mock_http_client, mock_response): + """Test making a PATCH request.""" + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint._make_request("PATCH", "test-endpoint", json={"key": "value"}) + + mock_http_client.patch.assert_called_once_with( + "https://api.example.com/test-endpoint", json={"key": "value"} + ) + assert result == mock_response + + def test_make_request_delete(self, mock_client, mock_http_client, mock_response): + """Test making a DELETE request.""" + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint._make_request("DELETE", "test-endpoint") + + mock_http_client.delete.assert_called_once_with( + "https://api.example.com/test-endpoint" + ) + assert result == mock_response + + def test_make_request_invalid_method(self, mock_client): + """Test making a request with an invalid HTTP method.""" + endpoint = MockService(mock_client) + + with pytest.raises(ValueError, match="Unsupported HTTP method: INVALID"): + endpoint._make_request("INVALID", "test-endpoint") + + def test_make_request_with_base_url_trailing_slash( + self, mock_client, mock_http_client, mock_response + ): + """Test making a request with a base URL that has a trailing slash.""" + mock_client.base_url = "https://api.example.com/" + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint._make_request("GET", "test-endpoint") + + mock_http_client.get.assert_called_once_with( + "https://api.example.com/test-endpoint" + ) + assert result == mock_response + + def test_make_request_with_path_leading_slash( + self, mock_client, mock_http_client, mock_response + ): + """Test making a request with a path that has a leading slash.""" + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint._make_request("GET", "/test-endpoint") + + mock_http_client.get.assert_called_once_with( + "https://api.example.com/test-endpoint" + ) + assert result == mock_response + + def test_list_resources(self, mock_client, mock_http_client, mock_response): + """Test list_resources method.""" + mock_response.status_code = 200 + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint.list_resources(page=1, per_page=10) + + mock_http_client.get.assert_called_once_with( + "https://api.example.com/mock-resources", page=1, per_page=10 + ) + assert isinstance(result, MockResourceResponse) + assert isinstance(result.data, list) + assert len(result.data) == 1 + assert result.data[0].id == "1" + assert result.data[0].type == "mock" + assert result.data[0].attributes["name"] == "test" + + def test_get_resource(self, mock_client, mock_http_client, mock_response): + """Test get_resource method.""" + mock_response.status_code = 200 + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint.get_resource("test-123") + + mock_http_client.get.assert_called_once_with( + "https://api.example.com/mock-resources/test-123" + ) + assert isinstance(result, MockResourceResponse) + assert isinstance(result.data, MockResourceData) + assert result.data.id == "test-123" + assert result.data.type == "mock" + assert result.data.attributes["name"] == "test" + + def test_create_resource(self, mock_client, mock_http_client, mock_response): + """Test create_resource method.""" + mock_response.status_code = 201 + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + data = {"name": "new-resource", "description": "A new resource"} + result = endpoint.create_resource(data) + + mock_http_client.post.assert_called_once_with( + "https://api.example.com/mock-resources", json=data + ) + assert isinstance(result, MockResourceResponse) + assert isinstance(result.data, MockResourceData) + assert result.data.id == "new" + assert result.data.type == "mock" + assert result.data.attributes["name"] == "new-resource" + assert result.data.attributes["description"] == "A new resource" + + def test_update_resource(self, mock_client, mock_http_client, mock_response): + """Test update_resource method.""" + mock_response.status_code = 200 + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + data = {"name": "updated-resource"} + result = endpoint.update_resource("test-123", data) + + mock_http_client.patch.assert_called_once_with( + "https://api.example.com/mock-resources/test-123", json=data + ) + assert isinstance(result, MockResourceResponse) + assert isinstance(result.data, MockResourceData) + assert result.data.id == "test-123" + assert result.data.type == "mock" + assert result.data.attributes["name"] == "updated-resource" + + def test_delete_resource(self, mock_client, mock_http_client, mock_response): + """Test delete_resource method.""" + mock_response.status_code = 204 + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint.delete_resource("test-123") + + mock_http_client.delete.assert_called_once_with( + "https://api.example.com/mock-resources/test-123" + ) + assert result is True + + def test_delete_resource_failure( + self, mock_client, mock_http_client, mock_response + ): + """Test delete_resource method when deletion fails.""" + mock_response.status_code = 404 + endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) + result = endpoint.delete_resource("test-123") + + mock_http_client.delete.assert_called_once_with( + "https://api.example.com/mock-resources/test-123" + ) + assert result is False \ No newline at end of file diff --git a/tfe/__init__.py b/tfe/__init__.py index 60fbccb2..91daabaf 100644 --- a/tfe/__init__.py +++ b/tfe/__init__.py @@ -8,5 +8,6 @@ from tfe.client import Client, TFEClientError from tfe.config import Config +from tfe.endpoint import Endpoint, ResourceDataProtocol, ResourceResponseProtocol -__all__ = ["Client", "TFEClientError", "Config"] +__all__ = ["Client", "TFEClientError", "Config", "Endpoint", "ResourceDataProtocol", "ResourceResponseProtocol"] diff --git a/tfe/endpoint.py b/tfe/endpoint.py new file mode 100644 index 00000000..901e1ee3 --- /dev/null +++ b/tfe/endpoint.py @@ -0,0 +1,170 @@ +""" +Base service class for Terraform Enterprise/Cloud API services. + +This module provides an abstract base class that all TFE API services +should inherit from. It provides common functionality for HTTP requests +and defines the interface that all service implementations must follow. +""" + +import logging +from abc import ABC, abstractmethod +from typing import Any, Protocol + +from requests import Response + +from tfe.client import Client + +logger = logging.getLogger(__name__) + + +class ResourceDataProtocol(Protocol): + """Protocol defining the structure of a single TFE resource in API response.""" + + id: str + type: str + attributes: dict[str, Any] + + +class ResourceResponseProtocol(Protocol): + """Protocol defining the structure of a TFE API response.""" + + data: ResourceDataProtocol | list[ResourceDataProtocol] + + +class Endpoint(ABC): + """Abstract base class for all TFE API services.""" + + def __init__(self, client: Client) -> None: + """ + Initialize the endpoint with a TFE client that provides HTTP access.""" + self._http_client = client.config.http_client + self._base_url = client.base_url + + def _make_request(self, method: str, path: str, **kwargs: Any) -> Response: + """ + Make an HTTP request using the client's HTTP client. + + Args: + method: HTTP method (GET, POST, PUT, PATCH, DELETE) + path: API path (will be joined with base_url) + **kwargs: Additional arguments to pass to the HTTP client + + Returns: + The HTTP response from requests library + """ + # Build full URL + url = self._base_url.rstrip("/") + "/" + path.lstrip("/") + + # Log the request directly here + logger.debug(f"Making {method.upper()} request to {path}") + + method = method.upper() + + # Make the request + if method == "GET": + return self._http_client.get(url, **kwargs) + elif method == "POST": + return self._http_client.post(url, **kwargs) + elif method == "PUT": + return self._http_client.put(url, **kwargs) + elif method == "PATCH": + return self._http_client.patch(url, **kwargs) + elif method == "DELETE": + return self._http_client.delete(url, **kwargs) + else: + raise ValueError(f"Unsupported HTTP method: {method}") + + @abstractmethod + def list_resources( + self, + page: int | None = None, + per_page: int | None = None, + search: str | None = None, + include: list[str] | None = None, + filter: dict[str, str] | None = None, + sort: str | None = None, + **additional_params: Any, + ) -> ResourceResponseProtocol: + """ + List resources instances. + + Args: + page: Page number for pagination (1-based) + per_page: Number of items per page + search: Search query string + include: List of related resources to include + filter: Filter criteria for resources + sort: Sort order for resources + **additional_params: Additional parameters specific to the endpoint + + Returns: + API response with list of resources in data field + """ + pass + + @abstractmethod + def get_resource( + self, + resource_id: str, + include: list[str] | None = None, + **additional_params: Any, + ) -> ResourceResponseProtocol: + """ + Get a single resource by ID. + + Args: + resource_id: The unique identifier of the resource + include: List of related resources to include + **additional_params: Additional parameters specific to the endpoint + + Returns: + API response with single resource in data field + """ + pass + + @abstractmethod + def create_resource( + self, data: dict[str, Any], **additional_params: Any + ) -> ResourceResponseProtocol: + """ + Create a new resource. + + Args: + data: Dictionary containing the resource data to create + **additional_params: Additional parameters specific to the endpoint + + Returns: + API response with newly created resource in data field + """ + pass + + @abstractmethod + def update_resource( + self, resource_id: str, data: dict[str, Any], **additional_params: Any + ) -> ResourceResponseProtocol: + """ + Update an existing resource. + + Args: + resource_id: The unique identifier of the resource to update + data: Dictionary containing the updated resource data + **additional_params: Additional parameters specific to the endpoint + + Returns: + API response with updated resource in data field + """ + pass + + @abstractmethod + def delete_resource(self, resource_id: str, **additional_params: Any) -> bool: + """ + Delete a resource. + + Args: + resource_id: The unique identifier of the resource to delete + **additional_params: Additional parameters specific to the endpoint + + Returns: + True if deletion was successful, False otherwise + """ + pass \ No newline at end of file From f9b5080df48ec55ee77db0f5eaface7f3e1a1be5 Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Fri, 29 Aug 2025 15:48:13 +0530 Subject: [PATCH 2/6] feat: base endpoint and response class --- tests/units/test_endpoint.py | 374 ++++++++++------------------------- tfe/__init__.py | 13 -- tfe/endpoint.py | 185 +++++------------ 3 files changed, 146 insertions(+), 426 deletions(-) diff --git a/tests/units/test_endpoint.py b/tests/units/test_endpoint.py index b910bbca..5c3057b6 100644 --- a/tests/units/test_endpoint.py +++ b/tests/units/test_endpoint.py @@ -1,304 +1,128 @@ -"""Unit tests for the Endpoint class.""" - -from typing import Any -from unittest.mock import Mock +"""Tests for the endpoint module.""" import pytest +from requests import Session +from requests.models import Response as RequestResponse -from tfe.endpoint import Endpoint, ResourceDataProtocol, ResourceResponseProtocol - - -class MockResourceData: - """Mock resource data for testing.""" - - def __init__(self, id: str, type: str, **attributes: Any): - self.id = id - self.type = type - self.attributes = attributes - - -class MockResourceResponse: - """Mock resource response for testing.""" +from tfe.endpoint import Endpoint - def __init__(self, data: ResourceDataProtocol | list[ResourceDataProtocol]): - self.data = data +@pytest.fixture +def mock_session(mocker): + """Create a mock session for testing.""" + return mocker.Mock(spec=Session) -# Mock service implementation for testing -class MockService(Endpoint): - """Mock service implementation for testing Endpoint.""" - def list_resources( - self, - page: int | None = None, - per_page: int | None = None, - search: str | None = None, - include: list[str] | None = None, - filter: dict[str, str] | None = None, - sort: str | None = None, - **additional_params: Any, - ) -> ResourceResponseProtocol: - """List resources.""" - # Build parameters dict - params = {} - if page is not None: - params["page"] = page - if per_page is not None: - params["per_page"] = per_page - if search is not None: - params["search"] = search - if include is not None: - params["include"] = include - if filter is not None: - params["filter"] = filter - if sort is not None: - params["sort"] = sort - # Add additional params - params.update(additional_params) +@pytest.fixture +def endpoint(mock_session): + """Create an Endpoint instance for testing.""" + return Endpoint(client=mock_session) - self._make_request("GET", "mock-resources", **params) - mock_data = MockResourceData(id="1", type="mock", name="test") - return MockResourceResponse(data=[mock_data]) - def get_resource( - self, - resource_id: str, - include: list[str] | None = None, - **additional_params: Any, - ) -> ResourceResponseProtocol: - """Get a single resource.""" - self._make_request("GET", f"mock-resources/{resource_id}", **additional_params) - mock_data = MockResourceData(id=resource_id, type="mock", name="test") - return MockResourceResponse(data=mock_data) - - def create_resource( - self, data: dict[str, Any], **additional_params: Any - ) -> ResourceResponseProtocol: - """Create a resource.""" - self._make_request("POST", "mock-resources", json=data) - mock_data = MockResourceData(id="new", type="mock", **data) - return MockResourceResponse(data=mock_data) - - def update_resource( - self, resource_id: str, data: dict[str, Any], **additional_params: Any - ) -> ResourceResponseProtocol: - """Update a resource.""" - self._make_request("PATCH", f"mock-resources/{resource_id}", json=data) - mock_data = MockResourceData(id=resource_id, type="mock", **data) - return MockResourceResponse(data=mock_data) - - def delete_resource(self, resource_id: str, **additional_params: Any) -> bool: - """Delete a resource.""" - response = self._make_request("DELETE", f"mock-resources/{resource_id}") - return response.status_code == 204 +@pytest.fixture +def mock_response(mocker): + """Create a mock HTTP response.""" + response = mocker.Mock(spec=RequestResponse) + response.status_code = 200 + response.json.return_value = {"data": "test"} + return response class TestEndpoint: - """Test the Endpoint class.""" - - @pytest.fixture - def mock_client(self): - """Create a mock client with standard configuration.""" - mock_client = Mock() - mock_client.config = Mock() - mock_client.config.http_client = Mock() - mock_client.base_url = "https://api.example.com" - return mock_client - - @pytest.fixture - def mock_http_client(self): - """Create a mock HTTP client.""" - return Mock() - - @pytest.fixture - def mock_response(self): - """Create a mock response.""" - return Mock() - - def _setup_endpoint(self, mock_client, mock_http_client, mock_response): - """Helper method to setup endpoint with mocked dependencies.""" - mock_http_client.get.return_value = mock_response - mock_http_client.post.return_value = mock_response - mock_http_client.put.return_value = mock_response - mock_http_client.patch.return_value = mock_response - mock_http_client.delete.return_value = mock_response - - mock_client.config.http_client = mock_http_client - return MockService(mock_client) - - def test_init(self, mock_client): - """Test endpoint initialization.""" - endpoint = MockService(mock_client) - - assert endpoint._http_client == mock_client.config.http_client - assert endpoint._base_url == "https://api.example.com" - - def test_make_request_get(self, mock_client, mock_http_client, mock_response): - """Test making a GET request.""" - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint._make_request("GET", "test-endpoint") - - mock_http_client.get.assert_called_once_with( - "https://api.example.com/test-endpoint" - ) - assert result == mock_response - - def test_make_request_post(self, mock_client, mock_http_client, mock_response): - """Test making a POST request.""" - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint._make_request("POST", "test-endpoint", json={"key": "value"}) - - mock_http_client.post.assert_called_once_with( - "https://api.example.com/test-endpoint", json={"key": "value"} - ) - assert result == mock_response - - def test_make_request_put(self, mock_client, mock_http_client, mock_response): - """Test making a PUT request.""" - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint._make_request("PUT", "test-endpoint", json={"key": "value"}) - - mock_http_client.put.assert_called_once_with( - "https://api.example.com/test-endpoint", json={"key": "value"} - ) - assert result == mock_response - - def test_make_request_patch(self, mock_client, mock_http_client, mock_response): - """Test making a PATCH request.""" - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint._make_request("PATCH", "test-endpoint", json={"key": "value"}) - - mock_http_client.patch.assert_called_once_with( - "https://api.example.com/test-endpoint", json={"key": "value"} - ) - assert result == mock_response - - def test_make_request_delete(self, mock_client, mock_http_client, mock_response): - """Test making a DELETE request.""" - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint._make_request("DELETE", "test-endpoint") - - mock_http_client.delete.assert_called_once_with( - "https://api.example.com/test-endpoint" - ) - assert result == mock_response - - def test_make_request_invalid_method(self, mock_client): - """Test making a request with an invalid HTTP method.""" - endpoint = MockService(mock_client) - + """Test cases for the Endpoint class.""" + + @pytest.mark.parametrize( + "method,expected_method,json_data,expected_call", + [ + ("GET", "get", None, ("get", "/test/path")), + ("POST", "post", {"key": "value"}, ("post", "/test/path")), + ("PUT", "put", {"key": "value"}, ("put", "/test/path")), + ("PATCH", "patch", {"key": "value"}, ("patch", "/test/path")), + ("DELETE", "delete", None, ("delete", "/test/path")), + ("get", "get", None, ("get", "/test/path")), # Test case insensitive + ( + "post", + "post", + {"data": "test"}, + ("post", "/test/path"), + ), # Test case insensitive + ], + ) + def test_make_request_all_methods( + self, endpoint, mock_response, method, expected_method, json_data, expected_call + ): + """Test _make_request with all supported HTTP methods.""" + # Setup the mock method to return our mock response + getattr(endpoint._http_client, expected_method).return_value = mock_response + + # Make the request + if json_data: + response = endpoint._make_request(method, "/test/path", json=json_data) + # Verify the correct method was called with the correct arguments + getattr(endpoint._http_client, expected_method).assert_called_once_with( + "/test/path", json=json_data + ) + else: + response = endpoint._make_request(method, "/test/path") + # Verify the correct method was called with the correct arguments + getattr(endpoint._http_client, expected_method).assert_called_once_with( + "/test/path" + ) + + # Verify the response is returned correctly + assert response == mock_response + + def test_make_request_unsupported_method(self, endpoint): + """Test that unsupported HTTP methods raise ValueError.""" with pytest.raises(ValueError, match="Unsupported HTTP method: INVALID"): - endpoint._make_request("INVALID", "test-endpoint") + endpoint._make_request("INVALID", "/test/path") - def test_make_request_with_base_url_trailing_slash( - self, mock_client, mock_http_client, mock_response - ): - """Test making a request with a base URL that has a trailing slash.""" - mock_client.base_url = "https://api.example.com/" - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint._make_request("GET", "test-endpoint") + def test_get_method(self, endpoint, mock_response): + """Test the _get convenience method.""" + endpoint._http_client.get.return_value = mock_response - mock_http_client.get.assert_called_once_with( - "https://api.example.com/test-endpoint" - ) - assert result == mock_response + response = endpoint._get("/test/path") - def test_make_request_with_path_leading_slash( - self, mock_client, mock_http_client, mock_response - ): - """Test making a request with a path that has a leading slash.""" - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint._make_request("GET", "/test-endpoint") + endpoint._http_client.get.assert_called_once_with("/test/path") + assert response == mock_response - mock_http_client.get.assert_called_once_with( - "https://api.example.com/test-endpoint" - ) - assert result == mock_response + def test_post_method(self, endpoint, mock_response): + """Test the _post convenience method.""" + endpoint._http_client.post.return_value = mock_response + test_data = {"key": "value"} - def test_list_resources(self, mock_client, mock_http_client, mock_response): - """Test list_resources method.""" - mock_response.status_code = 200 - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint.list_resources(page=1, per_page=10) + response = endpoint._post("/test/path", test_data) - mock_http_client.get.assert_called_once_with( - "https://api.example.com/mock-resources", page=1, per_page=10 - ) - assert isinstance(result, MockResourceResponse) - assert isinstance(result.data, list) - assert len(result.data) == 1 - assert result.data[0].id == "1" - assert result.data[0].type == "mock" - assert result.data[0].attributes["name"] == "test" + endpoint._http_client.post.assert_called_once_with("/test/path", json=test_data) + assert response == mock_response - def test_get_resource(self, mock_client, mock_http_client, mock_response): - """Test get_resource method.""" - mock_response.status_code = 200 - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint.get_resource("test-123") + def test_put_method(self, endpoint, mock_response): + """Test the _put convenience method.""" + endpoint._http_client.put.return_value = mock_response + test_data = {"key": "value"} - mock_http_client.get.assert_called_once_with( - "https://api.example.com/mock-resources/test-123" - ) - assert isinstance(result, MockResourceResponse) - assert isinstance(result.data, MockResourceData) - assert result.data.id == "test-123" - assert result.data.type == "mock" - assert result.data.attributes["name"] == "test" + response = endpoint._put("/test/path", test_data) - def test_create_resource(self, mock_client, mock_http_client, mock_response): - """Test create_resource method.""" - mock_response.status_code = 201 - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - data = {"name": "new-resource", "description": "A new resource"} - result = endpoint.create_resource(data) + endpoint._http_client.put.assert_called_once_with("/test/path", json=test_data) + assert response == mock_response - mock_http_client.post.assert_called_once_with( - "https://api.example.com/mock-resources", json=data - ) - assert isinstance(result, MockResourceResponse) - assert isinstance(result.data, MockResourceData) - assert result.data.id == "new" - assert result.data.type == "mock" - assert result.data.attributes["name"] == "new-resource" - assert result.data.attributes["description"] == "A new resource" + def test_patch_method(self, endpoint, mock_response): + """Test the _patch convenience method.""" + endpoint._http_client.patch.return_value = mock_response + test_data = {"key": "value"} - def test_update_resource(self, mock_client, mock_http_client, mock_response): - """Test update_resource method.""" - mock_response.status_code = 200 - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - data = {"name": "updated-resource"} - result = endpoint.update_resource("test-123", data) + response = endpoint._patch("/test/path", test_data) - mock_http_client.patch.assert_called_once_with( - "https://api.example.com/mock-resources/test-123", json=data + endpoint._http_client.patch.assert_called_once_with( + "/test/path", json=test_data ) - assert isinstance(result, MockResourceResponse) - assert isinstance(result.data, MockResourceData) - assert result.data.id == "test-123" - assert result.data.type == "mock" - assert result.data.attributes["name"] == "updated-resource" + assert response == mock_response - def test_delete_resource(self, mock_client, mock_http_client, mock_response): - """Test delete_resource method.""" - mock_response.status_code = 204 - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint.delete_resource("test-123") + def test_delete_method(self, endpoint, mock_response): + """Test the _delete convenience method.""" + endpoint._http_client.delete.return_value = mock_response - mock_http_client.delete.assert_called_once_with( - "https://api.example.com/mock-resources/test-123" - ) - assert result is True + response = endpoint._delete("/test/path") - def test_delete_resource_failure( - self, mock_client, mock_http_client, mock_response - ): - """Test delete_resource method when deletion fails.""" - mock_response.status_code = 404 - endpoint = self._setup_endpoint(mock_client, mock_http_client, mock_response) - result = endpoint.delete_resource("test-123") - - mock_http_client.delete.assert_called_once_with( - "https://api.example.com/mock-resources/test-123" - ) - assert result is False \ No newline at end of file + endpoint._http_client.delete.assert_called_once_with("/test/path") + assert response == mock_response diff --git a/tfe/__init__.py b/tfe/__init__.py index 91daabaf..e69de29b 100644 --- a/tfe/__init__.py +++ b/tfe/__init__.py @@ -1,13 +0,0 @@ -""" -Python client library for Terraform Enterprise/Cloud API. - -This package provides a Python interface to the Terraform Enterprise -and Terraform Cloud APIs, allowing you to programmatically manage -workspaces, runs, state files, and other TFE/TFC resources. -""" - -from tfe.client import Client, TFEClientError -from tfe.config import Config -from tfe.endpoint import Endpoint, ResourceDataProtocol, ResourceResponseProtocol - -__all__ = ["Client", "TFEClientError", "Config", "Endpoint", "ResourceDataProtocol", "ResourceResponseProtocol"] diff --git a/tfe/endpoint.py b/tfe/endpoint.py index 901e1ee3..0e770850 100644 --- a/tfe/endpoint.py +++ b/tfe/endpoint.py @@ -8,163 +8,72 @@ import logging from abc import ABC, abstractmethod -from typing import Any, Protocol +from typing import Any, TypeVar -from requests import Response - -from tfe.client import Client +from requests import Session +from requests.models import Response as RequestResponse logger = logging.getLogger(__name__) +T = TypeVar("T", bound="Response") -class ResourceDataProtocol(Protocol): - """Protocol defining the structure of a single TFE resource in API response.""" - - id: str - type: str - attributes: dict[str, Any] - - -class ResourceResponseProtocol(Protocol): - """Protocol defining the structure of a TFE API response.""" - data: ResourceDataProtocol | list[ResourceDataProtocol] +class Response(ABC): + @classmethod + @abstractmethod + def from_http_response(cls: type[T], response: RequestResponse) -> T: + """Create an instance of the endpoint specific response class from an HTTP response.""" + pass -class Endpoint(ABC): - """Abstract base class for all TFE API services.""" +class Endpoint: + """Base class for all TFE API services.""" - def __init__(self, client: Client) -> None: - """ - Initialize the endpoint with a TFE client that provides HTTP access.""" - self._http_client = client.config.http_client - self._base_url = client.base_url + def __init__(self, client: Session) -> None: + self._http_client = client - def _make_request(self, method: str, path: str, **kwargs: Any) -> Response: + def _make_request( + self, method: str, path: str, json: dict[str, Any] | None = None + ) -> RequestResponse: """ Make an HTTP request using the client's HTTP client. Args: method: HTTP method (GET, POST, PUT, PATCH, DELETE) - path: API path (will be joined with base_url) - **kwargs: Additional arguments to pass to the HTTP client - + path: API path + json: JSON payload for POST, PUT, PATCH requests Returns: The HTTP response from requests library """ # Build full URL - url = self._base_url.rstrip("/") + "/" + path.lstrip("/") - - # Log the request directly here - logger.debug(f"Making {method.upper()} request to {path}") - method = method.upper() # Make the request - if method == "GET": - return self._http_client.get(url, **kwargs) - elif method == "POST": - return self._http_client.post(url, **kwargs) - elif method == "PUT": - return self._http_client.put(url, **kwargs) - elif method == "PATCH": - return self._http_client.patch(url, **kwargs) - elif method == "DELETE": - return self._http_client.delete(url, **kwargs) - else: - raise ValueError(f"Unsupported HTTP method: {method}") - - @abstractmethod - def list_resources( - self, - page: int | None = None, - per_page: int | None = None, - search: str | None = None, - include: list[str] | None = None, - filter: dict[str, str] | None = None, - sort: str | None = None, - **additional_params: Any, - ) -> ResourceResponseProtocol: - """ - List resources instances. - - Args: - page: Page number for pagination (1-based) - per_page: Number of items per page - search: Search query string - include: List of related resources to include - filter: Filter criteria for resources - sort: Sort order for resources - **additional_params: Additional parameters specific to the endpoint - - Returns: - API response with list of resources in data field - """ - pass - - @abstractmethod - def get_resource( - self, - resource_id: str, - include: list[str] | None = None, - **additional_params: Any, - ) -> ResourceResponseProtocol: - """ - Get a single resource by ID. - - Args: - resource_id: The unique identifier of the resource - include: List of related resources to include - **additional_params: Additional parameters specific to the endpoint - - Returns: - API response with single resource in data field - """ - pass - - @abstractmethod - def create_resource( - self, data: dict[str, Any], **additional_params: Any - ) -> ResourceResponseProtocol: - """ - Create a new resource. - - Args: - data: Dictionary containing the resource data to create - **additional_params: Additional parameters specific to the endpoint - - Returns: - API response with newly created resource in data field - """ - pass - - @abstractmethod - def update_resource( - self, resource_id: str, data: dict[str, Any], **additional_params: Any - ) -> ResourceResponseProtocol: - """ - Update an existing resource. - - Args: - resource_id: The unique identifier of the resource to update - data: Dictionary containing the updated resource data - **additional_params: Additional parameters specific to the endpoint - - Returns: - API response with updated resource in data field - """ - pass - - @abstractmethod - def delete_resource(self, resource_id: str, **additional_params: Any) -> bool: - """ - Delete a resource. - - Args: - resource_id: The unique identifier of the resource to delete - **additional_params: Additional parameters specific to the endpoint - - Returns: - True if deletion was successful, False otherwise - """ - pass \ No newline at end of file + match method: + case "GET": + return self._http_client.get(path) + case "POST": + return self._http_client.post(path, json=json) + case "PUT": + return self._http_client.put(path, json=json) + case "PATCH": + return self._http_client.patch(path, json=json) + case "DELETE": + return self._http_client.delete(path) + case _: + raise ValueError(f"Unsupported HTTP method: {method}") + + def _get(self, path: str) -> RequestResponse: + return self._make_request("GET", path) + + def _post(self, path: str, data: dict) -> RequestResponse: + return self._make_request("POST", path, json=data) + + def _put(self, path: str, data: dict) -> RequestResponse: + return self._make_request("PUT", path, json=data) + + def _patch(self, path: str, data: dict) -> RequestResponse: + return self._make_request("PATCH", path, json=data) + + def _delete(self, path: str) -> RequestResponse: + return self._make_request("DELETE", path) From bcac43485e6b1baf67b396e1d7fdbb2074902d76 Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Fri, 29 Aug 2025 15:55:02 +0530 Subject: [PATCH 3/6] update comment Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tfe/endpoint.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tfe/endpoint.py b/tfe/endpoint.py index 0e770850..57e8d235 100644 --- a/tfe/endpoint.py +++ b/tfe/endpoint.py @@ -45,7 +45,6 @@ def _make_request( Returns: The HTTP response from requests library """ - # Build full URL method = method.upper() # Make the request From ab1f369cfce81f8cf1805919a363730cb4ff8c93 Mon Sep 17 00:00:00 2001 From: Sivaselvan32 Date: Tue, 2 Sep 2025 15:45:36 +0530 Subject: [PATCH 4/6] Added exceptional handling for http error --- tests/units/test_endpoint.py | 269 +++++++++++++++++++++++++++++++++- tests/units/test_exception.py | 95 ++++++++++++ tfe/endpoint.py | 252 +++++++++++++++++++++++++++---- tfe/exception.py | 88 +++++++++++ 4 files changed, 674 insertions(+), 30 deletions(-) create mode 100644 tests/units/test_exception.py create mode 100644 tfe/exception.py diff --git a/tests/units/test_endpoint.py b/tests/units/test_endpoint.py index 5c3057b6..7549aaee 100644 --- a/tests/units/test_endpoint.py +++ b/tests/units/test_endpoint.py @@ -1,10 +1,20 @@ """Tests for the endpoint module.""" import pytest -from requests import Session +from requests import Session, exceptions from requests.models import Response as RequestResponse from tfe.endpoint import Endpoint +from tfe.exception import ( + TFEConnectionException, + TFEEndpointException, + TFEForbiddenException, + TFENotFoundException, + TFEServerException, + TFETimeoutException, + TFEUnauthorizedException, + TFEValidationException, +) @pytest.fixture @@ -73,8 +83,11 @@ def test_make_request_all_methods( assert response == mock_response def test_make_request_unsupported_method(self, endpoint): - """Test that unsupported HTTP methods raise ValueError.""" - with pytest.raises(ValueError, match="Unsupported HTTP method: INVALID"): + """Test that unsupported HTTP methods raise TFEEndpointException.""" + with pytest.raises( + TFEEndpointException, + match="Unexpected error occurred during INVALID request", + ): endpoint._make_request("INVALID", "/test/path") def test_get_method(self, endpoint, mock_response): @@ -126,3 +139,253 @@ def test_delete_method(self, endpoint, mock_response): endpoint._http_client.delete.assert_called_once_with("/test/path") assert response == mock_response + + +class TestEndpointErrorHandling: + """Test cases for endpoint error handling.""" + + @pytest.mark.parametrize( + "handler,error_class,error_msg,error_type", + [ + ( + "_handle_connection_error", + TFEConnectionException, + "Failed to connect to TFE API", + exceptions.ConnectionError, + ), + ( + "_handle_timeout_error", + TFETimeoutException, + "Request timed out", + exceptions.Timeout, + ), + ( + "_handle_request_error", + TFEEndpointException, + "Request failed: Request failed", + exceptions.RequestException, + ), + ( + "_handle_unexpected_error", + TFEEndpointException, + "Unexpected error occurred during DELETE request", + ValueError, + ), + ], + ) + def test_error_handlers( + self, endpoint, handler, error_class, error_msg, error_type + ): + """Test all error handler methods.""" + method, path = "DELETE", "/test/path" + error = error_type("Request failed") + + with pytest.raises(error_class) as exc_info: + getattr(endpoint, handler)(method, path, error) + + exception = exc_info.value + assert exception.message == error_msg + assert exception.method == method + assert exception.path == path + assert exception.cause == error + + @pytest.mark.parametrize( + "response_data,expected", + [ + ({"error": "test"}, {"error": "test"}), + ({"text": "Error text"}, {"text": "Error text"}), + (None, None), + ], + ) + def test_extract_error_data(self, endpoint, mocker, response_data, expected): + """Test error data extraction from various response types.""" + if response_data is None: + result = endpoint._extract_error_data(None) + else: + mock_response = mocker.Mock() + if "error" in response_data: + mock_response.json.return_value = response_data + result = endpoint._extract_error_data(mock_response) + else: # text case + mock_response.json.side_effect = ValueError("Invalid JSON") + mock_response.text = response_data["text"] + result = endpoint._extract_error_data(mock_response) + + assert result == expected + + +class TestEndpointHTTPErrorHandling: + """Test cases for HTTP error handling.""" + + @pytest.mark.parametrize( + "status_code,exception_class,expected_message", + [ + ( + 401, + TFEUnauthorizedException, + "Authentication failed - invalid or missing token", + ), + (403, TFEForbiddenException, "Access forbidden - insufficient permissions"), + (404, TFENotFoundException, "Resource not found"), + (422, TFEValidationException, "Name is required; Email is invalid"), + (500, TFEServerException, "TFE server error"), + (418, TFEEndpointException, "HTTP error occurred"), # Unknown status + ], + ) + def test_handle_http_error_status_codes( + self, endpoint, mocker, status_code, exception_class, expected_message + ): + """Test HTTP error handling for different status codes.""" + method, path = "GET", "/test/path" + + mock_response = mocker.Mock() + mock_response.status_code = status_code + if status_code == 422: + mock_response.json.return_value = { + "errors": [ + {"detail": "Name is required"}, + {"detail": "Email is invalid"}, + ] + } + else: + mock_response.json.return_value = {"error": "Test error"} + + mock_http_error = exceptions.HTTPError(f"{status_code} Error") + mock_http_error.response = mock_response + + with pytest.raises(exception_class) as exc_info: + endpoint._handle_http_error(method, path, mock_http_error) + + exception = exc_info.value + assert exception.message == expected_message + assert exception.status_code == status_code + assert exception.method == method + assert exception.path == path + + def test_handle_http_error_no_response(self, endpoint): + """Test HTTP error handling with no response.""" + mock_http_error = exceptions.HTTPError("HTTP Error") + mock_http_error.response = None + + with pytest.raises(TFEEndpointException) as exc_info: + endpoint._handle_http_error("GET", "/test", mock_http_error) + + assert exc_info.value.message == "HTTP error occurred" + assert exc_info.value.status_code is None + + +class TestEndpointIntegration: + """Integration tests for endpoint error handling.""" + + @pytest.mark.parametrize( + "method,error_type,exception_class,expected_message", + [ + ( + "GET", + exceptions.ConnectionError, + TFEConnectionException, + "Failed to connect to TFE API", + ), + ("POST", exceptions.Timeout, TFETimeoutException, "Request timed out"), + ], + ) + def test_make_request_errors( + self, endpoint, method, error_type, exception_class, expected_message + ): + """Test _make_request with various error types.""" + path = "/test/path" + getattr(endpoint._http_client, method.lower()).side_effect = error_type( + "Test error" + ) + + with pytest.raises(exception_class) as exc_info: + endpoint._make_request(method, path) + + assert exc_info.value.message == expected_message + assert exc_info.value.method == method + assert exc_info.value.path == path + + def test_make_request_http_error(self, endpoint, mocker): + """Test _make_request with HTTP error.""" + mock_response = mocker.Mock() + mock_response.status_code = 401 + mock_response.json.return_value = {"error": "Unauthorized"} + mock_response.raise_for_status.side_effect = exceptions.HTTPError( + "401 Unauthorized" + ) + mock_response.raise_for_status.side_effect.response = mock_response + + endpoint._http_client.get.return_value = mock_response + + with pytest.raises(TFEUnauthorizedException) as exc_info: + endpoint._make_request("GET", "/test") + + assert exc_info.value.status_code == 401 + + def test_make_request_success(self, endpoint, mock_response): + """Test successful _make_request.""" + endpoint._http_client.get.return_value = mock_response + response = endpoint._make_request("GET", "/test") + assert response == mock_response + + +class TestEndpointErrorParsing: + """Test cases for error response parsing.""" + + @pytest.mark.parametrize( + "response_data,expected_message,expected_errors,expected_code", + [ + # JSON:API format with multiple errors + ( + { + "errors": [ + {"detail": "Name has already been taken"}, + {"detail": "Email is invalid"}, + ] + }, + "Name has already been taken; Email is invalid", + ["Name has already been taken", "Email is invalid"], + None, + ), + # JSON:API format with error code + ( + { + "errors": [ + {"detail": "Name is required", "code": "VALIDATION_ERROR"} + ] + }, + "Name is required", + ["Name is required"], + "VALIDATION_ERROR", + ), + # Simple message format + ({"message": "Resource not found"}, "Resource not found", [], None), + # Error field format + ( + {"error": "Invalid request parameters"}, + "Invalid request parameters", + [], + None, + ), + # Empty errors list + ({"errors": []}, "Unknown API error", [], None), + # Unknown format + ({"unknown_field": "some value"}, "Unknown API error", [], None), + # Malformed input + ("invalid json", "Unknown API error", [], None), + ], + ) + def test_parse_tfe_error_response_formats( + self, endpoint, response_data, expected_message, expected_errors, expected_code + ): + """Test parsing various error response formats.""" + result = endpoint.parse_tfe_error_response(response_data) + + assert result["message"] == expected_message + assert result["errors"] == expected_errors + assert result["error_code"] == expected_code + + def test_parse_tfe_error_response_none(self, endpoint): + """Test parsing None response.""" + result = endpoint.parse_tfe_error_response(None) + assert result["message"] == "Failed to parse error response: None" diff --git a/tests/units/test_exception.py b/tests/units/test_exception.py new file mode 100644 index 00000000..7461c574 --- /dev/null +++ b/tests/units/test_exception.py @@ -0,0 +1,95 @@ +"""Tests for the exception module.""" + +import pytest + +from tfe.exception import ( + TFEConnectionException, + TFEEndpointException, + TFEForbiddenException, + TFENotFoundException, + TFEServerException, + TFETimeoutException, + TFEUnauthorizedException, + TFEValidationException, +) + + +class TestTFEEndpointException: + """Test cases for TFEEndpointException base class.""" + + def test_exception_creation_and_properties(self): + """Test exception creation with various parameter combinations.""" + # Basic creation + basic = TFEEndpointException("Test message") + assert basic.message == "Test message" + assert basic.status_code is None + + # Full creation + cause = ValueError("Original error") + error_data = {"error": "test"} + full = TFEEndpointException( + message="Test message", + status_code=400, + error_data=error_data, + cause=cause, + method="GET", + path="/test/path", + ) + assert full.status_code == 400 + assert full.error_data == error_data + assert full.cause == cause + assert full.method == "GET" + assert full.path == "/test/path" + + def test_exception_string_representations(self): + """Test exception string and repr representations.""" + exception = TFEEndpointException( + message="Test message", + status_code=400, + method="GET", + path="/test/path", + error_data={"error": "test"}, + ) + + expected_str = ( + "Test message for GET /test/path (HTTP 400) (Error Data: {'error': 'test'})" + ) + expected_repr = "TFEEndpointException('Test message', status_code=400, method='GET', path='/test/path')" + + assert str(exception) == expected_str + assert repr(exception) == expected_repr + + def test_exception_inheritance(self): + """Test that TFEEndpointException inherits from Exception.""" + exception = TFEEndpointException("Test message") + assert isinstance(exception, Exception) + assert isinstance(exception, TFEEndpointException) + + +class TestCustomEndpointExceptions: + """Test cases for custom endpoint exception classes.""" + + @pytest.mark.parametrize( + "exception_class,status_code", + [ + (TFEConnectionException, None), + (TFETimeoutException, None), + (TFEUnauthorizedException, 401), + (TFEForbiddenException, 403), + (TFENotFoundException, 404), + (TFEValidationException, 422), + (TFEServerException, 500), + ], + ) + def test_custom_exceptions(self, exception_class, status_code): + """Test all custom exception classes.""" + exception = exception_class( + message="Test message", method="GET", path="/test", status_code=status_code + ) + + assert exception.message == "Test message" + assert exception.method == "GET" + assert exception.path == "/test" + assert exception.status_code == status_code + assert isinstance(exception, TFEEndpointException) + assert isinstance(exception, Exception) diff --git a/tfe/endpoint.py b/tfe/endpoint.py index 57e8d235..f1ef7d95 100644 --- a/tfe/endpoint.py +++ b/tfe/endpoint.py @@ -6,24 +6,25 @@ and defines the interface that all service implementations must follow. """ +import json import logging -from abc import ABC, abstractmethod -from typing import Any, TypeVar +from typing import Any, NoReturn -from requests import Session +from requests import Session, exceptions from requests.models import Response as RequestResponse -logger = logging.getLogger(__name__) - -T = TypeVar("T", bound="Response") - +from tfe.exception import ( + TFEConnectionException, + TFEEndpointException, + TFEForbiddenException, + TFENotFoundException, + TFEServerException, + TFETimeoutException, + TFEUnauthorizedException, + TFEValidationException, +) -class Response(ABC): - @classmethod - @abstractmethod - def from_http_response(cls: type[T], response: RequestResponse) -> T: - """Create an instance of the endpoint specific response class from an HTTP response.""" - pass +logger = logging.getLogger(__name__) class Endpoint: @@ -32,6 +33,180 @@ class Endpoint: def __init__(self, client: Session) -> None: self._http_client = client + def _handle_connection_error( + self, method: str, path: str, error: Exception + ) -> NoReturn: + """Handle connection-related errors.""" + logger.error( + "Connection error while making %s request to %s: %s", method, path, error + ) + raise TFEConnectionException( + message="Failed to connect to TFE API", + method=method, + path=path, + cause=error, + ) + + def _handle_timeout_error( + self, method: str, path: str, error: Exception + ) -> NoReturn: + """Handle timeout errors.""" + logger.error( + "Timeout error while making %s request to %s: %s", method, path, error + ) + raise TFETimeoutException( + message="Request timed out", method=method, path=path, cause=error + ) + + def _handle_http_error( + self, method: str, path: str, error: exceptions.HTTPError + ) -> NoReturn: + """Handle HTTP errors with specific status codes.""" + status_code = error.response.status_code if error.response else None + error_data = self._extract_error_data(error.response) + + logger.error( + "HTTP error while making %s request to %s: %s (Status: %s)", + method, + path, + error, + status_code, + ) + + # Map status code to specific exception + STATUS_CODE_MAPPING: dict[int, tuple[type[TFEEndpointException], str]] = { + 401: ( + TFEUnauthorizedException, + "Authentication failed - invalid or missing token", + ), + 403: (TFEForbiddenException, "Access forbidden - insufficient permissions"), + 404: (TFENotFoundException, "Resource not found"), + 422: (TFEValidationException, "Validation failed"), + } + + # Handle 5xx server errors + if status_code and 500 <= status_code < 600: + exception_class: type[TFEEndpointException] = TFEServerException + message = "TFE server error" + else: + # Get exception class and message from mapping, or use default + if status_code is not None: + exception_class, message = STATUS_CODE_MAPPING.get( + status_code, (TFEEndpointException, "HTTP error occurred") + ) + else: + exception_class, message = TFEEndpointException, "HTTP error occurred" + + # Special handling for validation errors (422) + if status_code == 422: + parsed_errors = ( + self.parse_tfe_error_response(error_data) if error_data else {} + ) + message = parsed_errors.get("message", message) + + raise exception_class( + message=message, + status_code=status_code, + error_data=error_data, + method=method, + path=path, + cause=error, + ) + + def _handle_request_error( + self, method: str, path: str, error: Exception + ) -> NoReturn: + """Handle general request errors.""" + logger.error( + "Request error while making %s request to %s: %s", method, path, error + ) + raise TFEEndpointException( + message=f"Request failed: {str(error)}", + method=method, + path=path, + cause=error, + ) + + def _handle_unexpected_error( + self, method: str, path: str, error: Exception + ) -> NoReturn: + """Handle unexpected errors.""" + logger.error( + "Unexpected error while making %s request to %s: %s", method, path, error + ) + raise TFEEndpointException( + message=f"Unexpected error occurred during {method} request", + method=method, + path=path, + cause=error, + ) + + def _extract_error_data( + self, response: RequestResponse | None + ) -> dict[str, Any] | None: + """Extract error data from HTTP response.""" + if not response: + return None + + try: + result = response.json() + return result if isinstance(result, dict) else {"text": str(result)} + except (ValueError, json.JSONDecodeError): + return {"text": response.text} + + def parse_tfe_error_response(self, response_data: dict[str, Any]) -> dict[str, Any]: + """ + Parse TFE API error response and extract meaningful error information. + + Args: + response_data: The JSON response from the TFE API + + Returns: + Dictionary containing parsed error information + """ + error_info: dict[str, Any] = { + "message": "Unknown API error", + "errors": [], + "error_code": None, + } + + try: + # Handle JSON:API error format + if "errors" in response_data: + errors = response_data["errors"] + if isinstance(errors, list) and errors: + # Extract error details + error_details = [] + for error in errors: + if isinstance(error, dict): + detail = error.get( + "detail", error.get("title", "Unknown error") + ) + error_details.append(detail) + + # Extract error code if available + if "code" in error and not error_info["error_code"]: + error_info["error_code"] = error["code"] + + error_info["errors"] = error_details + error_info["message"] = "; ".join( + str(detail) for detail in error_details + ) + + # Handle simple error message format + elif "message" in response_data: + error_info["message"] = response_data["message"] + + # Handle error field + elif "error" in response_data: + error_info["message"] = response_data["error"] + + except (KeyError, TypeError, AttributeError) as e: + logger.warning("Failed to parse error response: %s", e) + error_info["message"] = f"Failed to parse error response: {response_data}" + + return error_info + def _make_request( self, method: str, path: str, json: dict[str, Any] | None = None ) -> RequestResponse: @@ -46,21 +221,44 @@ def _make_request( The HTTP response from requests library """ method = method.upper() + response: RequestResponse | None = None + + try: + logger.debug("Making %s request to %s", method, path) + + # Make the request + match method: + case "GET": + response = self._http_client.get(path) + case "POST": + response = self._http_client.post(path, json=json) + case "PUT": + response = self._http_client.put(path, json=json) + case "PATCH": + response = self._http_client.patch(path, json=json) + case "DELETE": + response = self._http_client.delete(path) + case _: + raise TFEEndpointException( + message=f"Unsupported HTTP method: {method}", + method=method, + path=path, + ) + + # Check for HTTP errors and raise appropriate TFE exceptions + response.raise_for_status() + return response - # Make the request - match method: - case "GET": - return self._http_client.get(path) - case "POST": - return self._http_client.post(path, json=json) - case "PUT": - return self._http_client.put(path, json=json) - case "PATCH": - return self._http_client.patch(path, json=json) - case "DELETE": - return self._http_client.delete(path) - case _: - raise ValueError(f"Unsupported HTTP method: {method}") + except exceptions.ConnectionError as e: + self._handle_connection_error(method, path, e) + except exceptions.Timeout as e: + self._handle_timeout_error(method, path, e) + except exceptions.HTTPError as e: + self._handle_http_error(method, path, e) + except exceptions.RequestException as e: + self._handle_request_error(method, path, e) + except Exception as e: + self._handle_unexpected_error(method, path, e) def _get(self, path: str) -> RequestResponse: return self._make_request("GET", path) diff --git a/tfe/exception.py b/tfe/exception.py new file mode 100644 index 00000000..189dd7bc --- /dev/null +++ b/tfe/exception.py @@ -0,0 +1,88 @@ +from typing import Any + +class TFEEndpointException(Exception): + """Base exception for all TFE endpoint-related errors.""" + + def __init__( + self, + message: str, + status_code: int | None = None, + error_data: dict[str, Any] | None = None, + cause: Exception | None = None, + method: str | None = None, + path: str | None = None, + ) -> None: + self.message = message + self.status_code = status_code + self.error_data = error_data or {} + self.cause = cause + self.method = method + self.path = path + + # Build the full error message + full_message = self._build_error_message() + super().__init__(full_message) + + def _build_error_message(self) -> str: + """Build a comprehensive error message with request context.""" + parts = [self.message] + + if self.method and self.path: + parts.append(f"for {self.method} {self.path}") + + if self.status_code: + parts.append(f"(HTTP {self.status_code})") + + if self.error_data: + parts.append(f"(Error Data: {self.error_data})") + + return " ".join(parts) + + def __str__(self) -> str: + return self._build_error_message() + + def __repr__(self) -> str: + return f"{self.__class__.__name__}({self.message!r}, status_code={self.status_code}, method={self.method!r}, path={self.path!r})" + + +# Custom endpoint-specific exceptions +class TFEConnectionException(TFEEndpointException): + """Exception for connection-related errors.""" + + pass + + +class TFETimeoutException(TFEEndpointException): + """Exception for timeout errors.""" + + pass + + +class TFEUnauthorizedException(TFEEndpointException): + """Exception for 401 Unauthorized errors.""" + + pass + + +class TFEForbiddenException(TFEEndpointException): + """Exception for 403 Forbidden errors.""" + + pass + + +class TFENotFoundException(TFEEndpointException): + """Exception for 404 Not Found errors.""" + + pass + + +class TFEValidationException(TFEEndpointException): + """Exception for 422 Validation errors.""" + + pass + + +class TFEServerException(TFEEndpointException): + """Exception for 5xx server errors.""" + + pass From b1a58089c46741e87ff113fd9698db2319d7fd45 Mon Sep 17 00:00:00 2001 From: Sivaselvan32 Date: Tue, 2 Sep 2025 15:51:24 +0530 Subject: [PATCH 5/6] Updated ruff format --- tfe/exception.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tfe/exception.py b/tfe/exception.py index 189dd7bc..36823c61 100644 --- a/tfe/exception.py +++ b/tfe/exception.py @@ -1,5 +1,6 @@ from typing import Any + class TFEEndpointException(Exception): """Base exception for all TFE endpoint-related errors.""" From 8ac4189ace213ca19e31a9c84aed1e044a73f313 Mon Sep 17 00:00:00 2001 From: Sivaselvan32 Date: Wed, 3 Sep 2025 17:30:16 +0530 Subject: [PATCH 6/6] Reduced redundant testcases and Moved utils --- tests/units/test_endpoint.py | 292 +++++-------------------------- tests/units/test_error_utils.py | 293 ++++++++++++++++++++++++++++++++ tests/units/test_exception.py | 88 +++++++--- tfe/endpoint.py | 222 ++++-------------------- tfe/error_utils.py | 114 +++++++++++++ 5 files changed, 551 insertions(+), 458 deletions(-) create mode 100644 tests/units/test_error_utils.py create mode 100644 tfe/error_utils.py diff --git a/tests/units/test_endpoint.py b/tests/units/test_endpoint.py index 7549aaee..6185931e 100644 --- a/tests/units/test_endpoint.py +++ b/tests/units/test_endpoint.py @@ -8,12 +8,8 @@ from tfe.exception import ( TFEConnectionException, TFEEndpointException, - TFEForbiddenException, - TFENotFoundException, - TFEServerException, TFETimeoutException, TFEUnauthorizedException, - TFEValidationException, ) @@ -90,223 +86,81 @@ def test_make_request_unsupported_method(self, endpoint): ): endpoint._make_request("INVALID", "/test/path") - def test_get_method(self, endpoint, mock_response): - """Test the _get convenience method.""" - endpoint._http_client.get.return_value = mock_response - - response = endpoint._get("/test/path") - - endpoint._http_client.get.assert_called_once_with("/test/path") - assert response == mock_response - - def test_post_method(self, endpoint, mock_response): - """Test the _post convenience method.""" - endpoint._http_client.post.return_value = mock_response - test_data = {"key": "value"} - - response = endpoint._post("/test/path", test_data) - - endpoint._http_client.post.assert_called_once_with("/test/path", json=test_data) - assert response == mock_response - - def test_put_method(self, endpoint, mock_response): - """Test the _put convenience method.""" - endpoint._http_client.put.return_value = mock_response - test_data = {"key": "value"} - - response = endpoint._put("/test/path", test_data) - - endpoint._http_client.put.assert_called_once_with("/test/path", json=test_data) - assert response == mock_response - - def test_patch_method(self, endpoint, mock_response): - """Test the _patch convenience method.""" - endpoint._http_client.patch.return_value = mock_response - test_data = {"key": "value"} - - response = endpoint._patch("/test/path", test_data) - - endpoint._http_client.patch.assert_called_once_with( - "/test/path", json=test_data - ) - assert response == mock_response - - def test_delete_method(self, endpoint, mock_response): - """Test the _delete convenience method.""" - endpoint._http_client.delete.return_value = mock_response - - response = endpoint._delete("/test/path") - - endpoint._http_client.delete.assert_called_once_with("/test/path") - assert response == mock_response - class TestEndpointErrorHandling: """Test cases for endpoint error handling.""" @pytest.mark.parametrize( - "handler,error_class,error_msg,error_type", + "method,http_method,exception_class,request_exception,expected_message,expected_cause_type", [ ( - "_handle_connection_error", + "GET", + "get", TFEConnectionException, + exceptions.ConnectionError("Connection failed"), "Failed to connect to TFE API", exceptions.ConnectionError, ), ( - "_handle_timeout_error", + "POST", + "post", TFETimeoutException, + exceptions.Timeout("Request timed out"), "Request timed out", exceptions.Timeout, ), ( - "_handle_request_error", + "PUT", + "put", TFEEndpointException, + exceptions.RequestException("Request failed"), "Request failed: Request failed", exceptions.RequestException, ), ( - "_handle_unexpected_error", + "DELETE", + "delete", TFEEndpointException, + ValueError("Unexpected error"), "Unexpected error occurred during DELETE request", ValueError, ), ], ) - def test_error_handlers( - self, endpoint, handler, error_class, error_msg, error_type + def test_error_handling( + self, + endpoint, + method, + http_method, + exception_class, + request_exception, + expected_message, + expected_cause_type, ): - """Test all error handler methods.""" - method, path = "DELETE", "/test/path" - error = error_type("Request failed") - - with pytest.raises(error_class) as exc_info: - getattr(endpoint, handler)(method, path, error) - - exception = exc_info.value - assert exception.message == error_msg - assert exception.method == method - assert exception.path == path - assert exception.cause == error - - @pytest.mark.parametrize( - "response_data,expected", - [ - ({"error": "test"}, {"error": "test"}), - ({"text": "Error text"}, {"text": "Error text"}), - (None, None), - ], - ) - def test_extract_error_data(self, endpoint, mocker, response_data, expected): - """Test error data extraction from various response types.""" - if response_data is None: - result = endpoint._extract_error_data(None) - else: - mock_response = mocker.Mock() - if "error" in response_data: - mock_response.json.return_value = response_data - result = endpoint._extract_error_data(mock_response) - else: # text case - mock_response.json.side_effect = ValueError("Invalid JSON") - mock_response.text = response_data["text"] - result = endpoint._extract_error_data(mock_response) - - assert result == expected - - -class TestEndpointHTTPErrorHandling: - """Test cases for HTTP error handling.""" - - @pytest.mark.parametrize( - "status_code,exception_class,expected_message", - [ - ( - 401, - TFEUnauthorizedException, - "Authentication failed - invalid or missing token", - ), - (403, TFEForbiddenException, "Access forbidden - insufficient permissions"), - (404, TFENotFoundException, "Resource not found"), - (422, TFEValidationException, "Name is required; Email is invalid"), - (500, TFEServerException, "TFE server error"), - (418, TFEEndpointException, "HTTP error occurred"), # Unknown status - ], - ) - def test_handle_http_error_status_codes( - self, endpoint, mocker, status_code, exception_class, expected_message - ): - """Test HTTP error handling for different status codes.""" - method, path = "GET", "/test/path" - - mock_response = mocker.Mock() - mock_response.status_code = status_code - if status_code == 422: - mock_response.json.return_value = { - "errors": [ - {"detail": "Name is required"}, - {"detail": "Email is invalid"}, - ] - } - else: - mock_response.json.return_value = {"error": "Test error"} - - mock_http_error = exceptions.HTTPError(f"{status_code} Error") - mock_http_error.response = mock_response + """Test various error handling scenarios.""" + path = "/test/path" + getattr(endpoint._http_client, http_method).side_effect = request_exception with pytest.raises(exception_class) as exc_info: - endpoint._handle_http_error(method, path, mock_http_error) + endpoint._make_request(method, path) exception = exc_info.value assert exception.message == expected_message - assert exception.status_code == status_code assert exception.method == method assert exception.path == path + assert isinstance(exception.cause, expected_cause_type) - def test_handle_http_error_no_response(self, endpoint): - """Test HTTP error handling with no response.""" - mock_http_error = exceptions.HTTPError("HTTP Error") - mock_http_error.response = None - - with pytest.raises(TFEEndpointException) as exc_info: - endpoint._handle_http_error("GET", "/test", mock_http_error) - - assert exc_info.value.message == "HTTP error occurred" - assert exc_info.value.status_code is None - - -class TestEndpointIntegration: - """Integration tests for endpoint error handling.""" + def test_http_error_handling(self, endpoint, mocker): + """Test HTTP error handling (delegated to error_utils).""" + method, path = "GET", "/test/path" - @pytest.mark.parametrize( - "method,error_type,exception_class,expected_message", - [ - ( - "GET", - exceptions.ConnectionError, - TFEConnectionException, - "Failed to connect to TFE API", - ), - ("POST", exceptions.Timeout, TFETimeoutException, "Request timed out"), - ], - ) - def test_make_request_errors( - self, endpoint, method, error_type, exception_class, expected_message - ): - """Test _make_request with various error types.""" - path = "/test/path" - getattr(endpoint._http_client, method.lower()).side_effect = error_type( - "Test error" + # Mock the handle_http_error function to raise an exception + mock_handle_http_error = mocker.patch("tfe.endpoint.handle_http_error") + mock_handle_http_error.side_effect = TFEUnauthorizedException( + message="Authentication failed", status_code=401, method=method, path=path ) - with pytest.raises(exception_class) as exc_info: - endpoint._make_request(method, path) - - assert exc_info.value.message == expected_message - assert exc_info.value.method == method - assert exc_info.value.path == path - - def test_make_request_http_error(self, endpoint, mocker): - """Test _make_request with HTTP error.""" + # Create mock response that raises HTTPError mock_response = mocker.Mock() mock_response.status_code = 401 mock_response.json.return_value = {"error": "Unauthorized"} @@ -317,75 +171,9 @@ def test_make_request_http_error(self, endpoint, mocker): endpoint._http_client.get.return_value = mock_response - with pytest.raises(TFEUnauthorizedException) as exc_info: - endpoint._make_request("GET", "/test") - - assert exc_info.value.status_code == 401 - - def test_make_request_success(self, endpoint, mock_response): - """Test successful _make_request.""" - endpoint._http_client.get.return_value = mock_response - response = endpoint._make_request("GET", "/test") - assert response == mock_response - - -class TestEndpointErrorParsing: - """Test cases for error response parsing.""" - - @pytest.mark.parametrize( - "response_data,expected_message,expected_errors,expected_code", - [ - # JSON:API format with multiple errors - ( - { - "errors": [ - {"detail": "Name has already been taken"}, - {"detail": "Email is invalid"}, - ] - }, - "Name has already been taken; Email is invalid", - ["Name has already been taken", "Email is invalid"], - None, - ), - # JSON:API format with error code - ( - { - "errors": [ - {"detail": "Name is required", "code": "VALIDATION_ERROR"} - ] - }, - "Name is required", - ["Name is required"], - "VALIDATION_ERROR", - ), - # Simple message format - ({"message": "Resource not found"}, "Resource not found", [], None), - # Error field format - ( - {"error": "Invalid request parameters"}, - "Invalid request parameters", - [], - None, - ), - # Empty errors list - ({"errors": []}, "Unknown API error", [], None), - # Unknown format - ({"unknown_field": "some value"}, "Unknown API error", [], None), - # Malformed input - ("invalid json", "Unknown API error", [], None), - ], - ) - def test_parse_tfe_error_response_formats( - self, endpoint, response_data, expected_message, expected_errors, expected_code - ): - """Test parsing various error response formats.""" - result = endpoint.parse_tfe_error_response(response_data) - - assert result["message"] == expected_message - assert result["errors"] == expected_errors - assert result["error_code"] == expected_code + # This should call handle_http_error and raise TFEUnauthorizedException + with pytest.raises(TFEUnauthorizedException): + endpoint._make_request(method, path) - def test_parse_tfe_error_response_none(self, endpoint): - """Test parsing None response.""" - result = endpoint.parse_tfe_error_response(None) - assert result["message"] == "Failed to parse error response: None" + # Verify that handle_http_error was called + mock_handle_http_error.assert_called_once() diff --git a/tests/units/test_error_utils.py b/tests/units/test_error_utils.py new file mode 100644 index 00000000..433dc60e --- /dev/null +++ b/tests/units/test_error_utils.py @@ -0,0 +1,293 @@ +"""Tests for the error_utils module.""" + +from unittest.mock import Mock + +import pytest +from requests import exceptions + +from tfe.error_utils import ( + extract_error_data, + handle_http_error, + parse_tfe_error_response, +) +from tfe.exception import ( + TFEEndpointException, + TFEForbiddenException, + TFENotFoundException, + TFEServerException, + TFEUnauthorizedException, + TFEValidationException, +) + + +class TestExtractErrorData: + """Test cases for extract_error_data function.""" + + def test_extract_none_response(self): + """Test handling None response.""" + result = extract_error_data(None) + assert result is None + + @pytest.mark.parametrize( + "json_return_value,json_side_effect,text_value,expected_result", + [ + # Test extracting JSON error data + ({"error": "test error"}, None, None, {"error": "test error"}), + # Test extracting text when JSON fails + (None, ValueError("Invalid JSON"), "Error text", {"text": "Error text"}), + # Test handling non-dict JSON response + ("simple string", None, None, {"text": "simple string"}), + # Test handling JSON decode error + ( + None, + ValueError("JSON decode error"), + "Raw response text", + {"text": "Raw response text"}, + ), + ], + ) + def test_extract_data_scenarios( + self, json_return_value, json_side_effect, text_value, expected_result + ): + """Test various data extraction scenarios.""" + mock_response = Mock() + mock_response.json.return_value = json_return_value + if json_side_effect: + mock_response.json.side_effect = json_side_effect + if text_value: + mock_response.text = text_value + + result = extract_error_data(mock_response) + + assert result == expected_result + if json_return_value is not None and json_side_effect is None: + mock_response.json.assert_called_once() + + +class TestParseTfeErrorResponse: + """Test cases for parse_tfe_error_response function.""" + + @pytest.mark.parametrize( + "response_data,expected_message,expected_errors,expected_code", + [ + # JSON:API format with multiple errors + ( + { + "errors": [ + {"detail": "Name has already been taken"}, + {"detail": "Email is invalid"}, + ] + }, + "Name has already been taken; Email is invalid", + ["Name has already been taken", "Email is invalid"], + None, + ), + # JSON:API format with error code + ( + { + "errors": [ + {"detail": "Name is required", "code": "VALIDATION_ERROR"} + ] + }, + "Name is required", + ["Name is required"], + "VALIDATION_ERROR", + ), + # JSON:API format with title fallback + ( + {"errors": [{"title": "Validation Failed", "code": "INVALID_DATA"}]}, + "Validation Failed", + ["Validation Failed"], + "INVALID_DATA", + ), + # Simple message format + ({"message": "Resource not found"}, "Resource not found", [], None), + # Error field format + ( + {"error": "Invalid request parameters"}, + "Invalid request parameters", + [], + None, + ), + # Empty errors list + ({"errors": []}, "Unknown API error", [], None), + # Unknown format + ({"unknown_field": "some value"}, "Unknown API error", [], None), + # Malformed response (string) + ("invalid json", "Unknown API error", [], None), + # None response + (None, "Failed to parse error response: None", [], None), + # Multiple error codes (should use first one) + ( + { + "errors": [ + {"detail": "First error", "code": "FIRST_CODE"}, + {"detail": "Second error", "code": "SECOND_CODE"}, + ] + }, + "First error; Second error", + ["First error", "Second error"], + "FIRST_CODE", + ), + # Non-dict error objects + ( + {"errors": ["Simple string error", {"detail": "Dict error"}]}, + "Dict error", + ["Dict error"], + None, + ), + ], + ) + def test_parse_various_formats( + self, response_data, expected_message, expected_errors, expected_code + ): + """Test parsing various error response formats.""" + result = parse_tfe_error_response(response_data) + + assert result["message"] == expected_message + assert result["errors"] == expected_errors + assert result["error_code"] == expected_code + + +class TestHandleHttpError: + """Test cases for handle_http_error function.""" + + @pytest.mark.parametrize( + "status_code,expected_exception_class,expected_message", + [ + ( + 401, + TFEUnauthorizedException, + "Authentication failed - invalid or missing token", + ), + (403, TFEForbiddenException, "Access forbidden - insufficient permissions"), + (404, TFENotFoundException, "Resource not found"), + (422, TFEValidationException, "Name is required; Email is invalid"), + (500, TFEServerException, "TFE server error"), + (502, TFEServerException, "TFE server error"), + (503, TFEServerException, "TFE server error"), + (504, TFEServerException, "TFE server error"), + (418, TFEEndpointException, "HTTP error occurred"), # Unknown status + ], + ) + def test_status_code_mapping( + self, status_code, expected_exception_class, expected_message + ): + """Test that status codes map to correct exception types.""" + method, path = "GET", "/test/path" + + mock_response = Mock() + mock_response.status_code = status_code + if status_code == 422: + mock_response.json.return_value = { + "errors": [ + {"detail": "Name is required"}, + {"detail": "Email is invalid"}, + ] + } + else: + mock_response.json.return_value = {"error": "Test error"} + + mock_http_error = exceptions.HTTPError(f"{status_code} Error") + mock_http_error.response = mock_response + + with pytest.raises(expected_exception_class) as exc_info: + handle_http_error(method, path, mock_http_error) + + exception = exc_info.value + assert exception.message == expected_message + assert exception.status_code == status_code + assert exception.method == method + assert exception.path == path + assert exception.cause == mock_http_error + + @pytest.mark.parametrize( + "method,path,json_response,expected_message", + [ + # Test 422 validation error with parsed error message + ( + "POST", + "/api/workspaces", + { + "errors": [ + {"detail": "Name is required"}, + {"detail": "Email format is invalid"}, + ] + }, + "Name is required; Email format is invalid", + ), + # Test 422 validation error with fallback message when parsing fails + ( + "POST", + "/api/workspaces", + {"unknown_format": "data"}, + "Unknown API error", # Actual fallback message from parse_tfe_error_response + ), + ], + ) + def test_validation_error_scenarios( + self, method, path, json_response, expected_message + ): + """Test various 422 validation error scenarios.""" + mock_response = Mock() + mock_response.status_code = 422 + mock_response.json.return_value = json_response + + mock_http_error = exceptions.HTTPError("422 Validation Error") + mock_http_error.response = mock_response + + with pytest.raises(TFEValidationException) as exc_info: + handle_http_error(method, path, mock_http_error) + + exception = exc_info.value + assert exception.message == expected_message + assert exception.status_code == 422 + assert exception.error_data == json_response + + def test_error_data_preservation(self): + """Test that error data is preserved in the exception.""" + method, path = "GET", "/test" + + mock_response = Mock() + mock_response.status_code = 400 + mock_response.json.return_value = { + "error": "Bad Request", + "details": {"field": "value"}, + } + + mock_http_error = exceptions.HTTPError("400 Bad Request") + mock_http_error.response = mock_response + + with pytest.raises(TFEEndpointException) as exc_info: + handle_http_error(method, path, mock_http_error) + + exception = exc_info.value + assert exception.error_data == mock_response.json.return_value + assert exception.error_data["error"] == "Bad Request" + assert exception.error_data["details"]["field"] == "value" + + def test_logging_behavior(self, mocker): + """Test that appropriate logging occurs.""" + mock_logger = mocker.patch("tfe.error_utils.logger") + + method, path = "GET", "/test" + mock_response = Mock() + mock_response.status_code = 404 + mock_response.json.return_value = {"error": "Not found"} + + mock_http_error = exceptions.HTTPError("404 Not Found") + mock_http_error.response = mock_response + + with pytest.raises(TFENotFoundException): + handle_http_error(method, path, mock_http_error) + + # Verify that error logging occurred + mock_logger.error.assert_called_once() + log_call_args = mock_logger.error.call_args[0] + assert "HTTP error while making" in log_call_args[0] + assert method in log_call_args[1] + assert path in log_call_args[2] + assert ( + log_call_args[3] == mock_http_error + ) # The error object is passed directly + assert log_call_args[4] == 404 # Status code is an integer diff --git a/tests/units/test_exception.py b/tests/units/test_exception.py index 7461c574..79781954 100644 --- a/tests/units/test_exception.py +++ b/tests/units/test_exception.py @@ -19,11 +19,6 @@ class TestTFEEndpointException: def test_exception_creation_and_properties(self): """Test exception creation with various parameter combinations.""" - # Basic creation - basic = TFEEndpointException("Test message") - assert basic.message == "Test message" - assert basic.status_code is None - # Full creation cause = ValueError("Original error") error_data = {"error": "test"} @@ -41,23 +36,13 @@ def test_exception_creation_and_properties(self): assert full.method == "GET" assert full.path == "/test/path" - def test_exception_string_representations(self): - """Test exception string and repr representations.""" - exception = TFEEndpointException( - message="Test message", - status_code=400, - method="GET", - path="/test/path", - error_data={"error": "test"}, - ) - + # Test string and repr representation expected_str = ( "Test message for GET /test/path (HTTP 400) (Error Data: {'error': 'test'})" ) expected_repr = "TFEEndpointException('Test message', status_code=400, method='GET', path='/test/path')" - - assert str(exception) == expected_str - assert repr(exception) == expected_repr + assert str(full) == expected_str + assert repr(full) == expected_repr def test_exception_inheritance(self): """Test that TFEEndpointException inherits from Exception.""" @@ -65,6 +50,55 @@ def test_exception_inheritance(self): assert isinstance(exception, Exception) assert isinstance(exception, TFEEndpointException) + @pytest.mark.parametrize( + "message,status_code,method,path,error_data,expected_str", + [ + # Test with minimal info + ("Simple error", None, None, None, None, "Simple error"), + # Test with method and path + ( + "API error", + None, + "POST", + "/api/test", + None, + "API error for POST /api/test", + ), + # Test with status code + ("HTTP error", 404, None, None, None, "HTTP error (HTTP 404)"), + # Test with error data + ( + "Validation error", + None, + None, + None, + {"field": "name"}, + "Validation error (Error Data: {'field': 'name'})", + ), + # Test with all components + ( + "Complete error", + 422, + "PUT", + "/api/workspaces", + {"errors": ["Name is required"]}, + "Complete error for PUT /api/workspaces (HTTP 422) (Error Data: {'errors': ['Name is required']})", + ), + ], + ) + def test_error_message_building( + self, message, status_code, method, path, error_data, expected_str + ): + """Test error message building with different combinations.""" + exception = TFEEndpointException( + message=message, + status_code=status_code, + method=method, + path=path, + error_data=error_data, + ) + assert str(exception) == expected_str + class TestCustomEndpointExceptions: """Test cases for custom endpoint exception classes.""" @@ -88,8 +122,22 @@ def test_custom_exceptions(self, exception_class, status_code): ) assert exception.message == "Test message" - assert exception.method == "GET" - assert exception.path == "/test" assert exception.status_code == status_code assert isinstance(exception, TFEEndpointException) assert isinstance(exception, Exception) + + def test_exception_with_error_data(self): + """Test exception with error data.""" + error_data = { + "errors": [{"detail": "Name is required"}, {"detail": "Email is invalid"}] + } + exception = TFEValidationException( + message="Validation failed", + status_code=422, + method="POST", + path="/api/workspaces", + error_data=error_data, + ) + + assert exception.error_data == error_data + assert exception.error_data["errors"][0]["detail"] == "Name is required" diff --git a/tfe/endpoint.py b/tfe/endpoint.py index f1ef7d95..37720612 100644 --- a/tfe/endpoint.py +++ b/tfe/endpoint.py @@ -6,22 +6,17 @@ and defines the interface that all service implementations must follow. """ -import json import logging -from typing import Any, NoReturn +from typing import Any from requests import Session, exceptions from requests.models import Response as RequestResponse +from tfe.error_utils import handle_http_error from tfe.exception import ( TFEConnectionException, TFEEndpointException, - TFEForbiddenException, - TFENotFoundException, - TFEServerException, TFETimeoutException, - TFEUnauthorizedException, - TFEValidationException, ) logger = logging.getLogger(__name__) @@ -33,180 +28,6 @@ class Endpoint: def __init__(self, client: Session) -> None: self._http_client = client - def _handle_connection_error( - self, method: str, path: str, error: Exception - ) -> NoReturn: - """Handle connection-related errors.""" - logger.error( - "Connection error while making %s request to %s: %s", method, path, error - ) - raise TFEConnectionException( - message="Failed to connect to TFE API", - method=method, - path=path, - cause=error, - ) - - def _handle_timeout_error( - self, method: str, path: str, error: Exception - ) -> NoReturn: - """Handle timeout errors.""" - logger.error( - "Timeout error while making %s request to %s: %s", method, path, error - ) - raise TFETimeoutException( - message="Request timed out", method=method, path=path, cause=error - ) - - def _handle_http_error( - self, method: str, path: str, error: exceptions.HTTPError - ) -> NoReturn: - """Handle HTTP errors with specific status codes.""" - status_code = error.response.status_code if error.response else None - error_data = self._extract_error_data(error.response) - - logger.error( - "HTTP error while making %s request to %s: %s (Status: %s)", - method, - path, - error, - status_code, - ) - - # Map status code to specific exception - STATUS_CODE_MAPPING: dict[int, tuple[type[TFEEndpointException], str]] = { - 401: ( - TFEUnauthorizedException, - "Authentication failed - invalid or missing token", - ), - 403: (TFEForbiddenException, "Access forbidden - insufficient permissions"), - 404: (TFENotFoundException, "Resource not found"), - 422: (TFEValidationException, "Validation failed"), - } - - # Handle 5xx server errors - if status_code and 500 <= status_code < 600: - exception_class: type[TFEEndpointException] = TFEServerException - message = "TFE server error" - else: - # Get exception class and message from mapping, or use default - if status_code is not None: - exception_class, message = STATUS_CODE_MAPPING.get( - status_code, (TFEEndpointException, "HTTP error occurred") - ) - else: - exception_class, message = TFEEndpointException, "HTTP error occurred" - - # Special handling for validation errors (422) - if status_code == 422: - parsed_errors = ( - self.parse_tfe_error_response(error_data) if error_data else {} - ) - message = parsed_errors.get("message", message) - - raise exception_class( - message=message, - status_code=status_code, - error_data=error_data, - method=method, - path=path, - cause=error, - ) - - def _handle_request_error( - self, method: str, path: str, error: Exception - ) -> NoReturn: - """Handle general request errors.""" - logger.error( - "Request error while making %s request to %s: %s", method, path, error - ) - raise TFEEndpointException( - message=f"Request failed: {str(error)}", - method=method, - path=path, - cause=error, - ) - - def _handle_unexpected_error( - self, method: str, path: str, error: Exception - ) -> NoReturn: - """Handle unexpected errors.""" - logger.error( - "Unexpected error while making %s request to %s: %s", method, path, error - ) - raise TFEEndpointException( - message=f"Unexpected error occurred during {method} request", - method=method, - path=path, - cause=error, - ) - - def _extract_error_data( - self, response: RequestResponse | None - ) -> dict[str, Any] | None: - """Extract error data from HTTP response.""" - if not response: - return None - - try: - result = response.json() - return result if isinstance(result, dict) else {"text": str(result)} - except (ValueError, json.JSONDecodeError): - return {"text": response.text} - - def parse_tfe_error_response(self, response_data: dict[str, Any]) -> dict[str, Any]: - """ - Parse TFE API error response and extract meaningful error information. - - Args: - response_data: The JSON response from the TFE API - - Returns: - Dictionary containing parsed error information - """ - error_info: dict[str, Any] = { - "message": "Unknown API error", - "errors": [], - "error_code": None, - } - - try: - # Handle JSON:API error format - if "errors" in response_data: - errors = response_data["errors"] - if isinstance(errors, list) and errors: - # Extract error details - error_details = [] - for error in errors: - if isinstance(error, dict): - detail = error.get( - "detail", error.get("title", "Unknown error") - ) - error_details.append(detail) - - # Extract error code if available - if "code" in error and not error_info["error_code"]: - error_info["error_code"] = error["code"] - - error_info["errors"] = error_details - error_info["message"] = "; ".join( - str(detail) for detail in error_details - ) - - # Handle simple error message format - elif "message" in response_data: - error_info["message"] = response_data["message"] - - # Handle error field - elif "error" in response_data: - error_info["message"] = response_data["error"] - - except (KeyError, TypeError, AttributeError) as e: - logger.warning("Failed to parse error response: %s", e) - error_info["message"] = f"Failed to parse error response: {response_data}" - - return error_info - def _make_request( self, method: str, path: str, json: dict[str, Any] | None = None ) -> RequestResponse: @@ -250,15 +71,44 @@ def _make_request( return response except exceptions.ConnectionError as e: - self._handle_connection_error(method, path, e) + logger.error( + "Connection error while making %s request to %s: %s", method, path, e + ) + raise TFEConnectionException( + message="Failed to connect to TFE API", + method=method, + path=path, + cause=e, + ) from e except exceptions.Timeout as e: - self._handle_timeout_error(method, path, e) + logger.error( + "Timeout error while making %s request to %s: %s", method, path, e + ) + raise TFETimeoutException( + message="Request timed out", method=method, path=path, cause=e + ) from e except exceptions.HTTPError as e: - self._handle_http_error(method, path, e) + handle_http_error(method, path, e) except exceptions.RequestException as e: - self._handle_request_error(method, path, e) + logger.error( + "Request error while making %s request to %s: %s", method, path, e + ) + raise TFEEndpointException( + message=f"Request failed: {str(e)}", + method=method, + path=path, + cause=e, + ) from e except Exception as e: - self._handle_unexpected_error(method, path, e) + logger.error( + "Unexpected error while making %s request to %s: %s", method, path, e + ) + raise TFEEndpointException( + message=f"Unexpected error occurred during {method} request", + method=method, + path=path, + cause=e, + ) from e def _get(self, path: str) -> RequestResponse: return self._make_request("GET", path) diff --git a/tfe/error_utils.py b/tfe/error_utils.py new file mode 100644 index 00000000..3800efdf --- /dev/null +++ b/tfe/error_utils.py @@ -0,0 +1,114 @@ +""" +Utility functions for handling HTTP errors and parsing error responses in TFE API client. +""" + +import json +import logging +from typing import Any, NoReturn + +from requests import exceptions +from requests.models import Response as RequestResponse + +from tfe.exception import ( + TFEEndpointException, + TFEForbiddenException, + TFENotFoundException, + TFEServerException, + TFEUnauthorizedException, + TFEValidationException, +) + +logger = logging.getLogger(__name__) + + +def extract_error_data(response: RequestResponse | None) -> dict[str, Any] | None: + """Extract error data from HTTP response.""" + if not response: + return None + try: + result = response.json() + return result if isinstance(result, dict) else {"text": str(result)} + except (ValueError, json.JSONDecodeError): + return {"text": response.text} + + +def parse_tfe_error_response(response_data: dict[str, Any]) -> dict[str, Any]: + """ + Parse TFE API error response and extract meaningful error information. + """ + error_info: dict[str, Any] = { + "message": "Unknown API error", + "errors": [], + "error_code": None, + } + try: + if "errors" in response_data: + errors = response_data["errors"] + if isinstance(errors, list) and errors: + error_details = [] + for error in errors: + if isinstance(error, dict): + detail = error.get( + "detail", error.get("title", "Unknown error") + ) + error_details.append(detail) + if "code" in error and not error_info["error_code"]: + error_info["error_code"] = error["code"] + error_info["errors"] = error_details + error_info["message"] = "; ".join( + str(detail) for detail in error_details + ) + elif "message" in response_data: + error_info["message"] = response_data["message"] + elif "error" in response_data: + error_info["message"] = response_data["error"] + except (KeyError, TypeError, AttributeError) as e: + logger.warning("Failed to parse error response: %s", e) + error_info["message"] = f"Failed to parse error response: {response_data}" + return error_info + + +def handle_http_error(method: str, path: str, error: exceptions.HTTPError) -> NoReturn: + """ + Handle HTTP errors with specific status codes and raise appropriate exceptions. + """ + status_code = error.response.status_code if error.response else None + error_data = extract_error_data(error.response) + logger.error( + "HTTP error while making %s request to %s: %s (Status: %s)", + method, + path, + error, + status_code, + ) + STATUS_CODE_MAPPING: dict[int, tuple[type[TFEEndpointException], str]] = { + 401: ( + TFEUnauthorizedException, + "Authentication failed - invalid or missing token", + ), + 403: (TFEForbiddenException, "Access forbidden - insufficient permissions"), + 404: (TFENotFoundException, "Resource not found"), + 422: (TFEValidationException, "Validation failed"), + } + exception_class: type[TFEEndpointException] + if status_code and 500 <= status_code < 600: + exception_class = TFEServerException + message = "TFE server error" + else: + if status_code is not None: + exception_class, message = STATUS_CODE_MAPPING.get( + status_code, (TFEEndpointException, "HTTP error occurred") + ) + else: + exception_class, message = TFEEndpointException, "HTTP error occurred" + if status_code == 422: + parsed_errors = parse_tfe_error_response(error_data) if error_data else {} + message = parsed_errors.get("message", message) + raise exception_class( + message=message, + status_code=status_code, + error_data=error_data, + method=method, + path=path, + cause=error, + ) from error