diff --git a/docs/api/rest-api.md b/docs/api/rest-api.md index 1a11040..a6c5dc4 100644 --- a/docs/api/rest-api.md +++ b/docs/api/rest-api.md @@ -316,8 +316,8 @@ Errors follow RFC 7807 Problem Details: { "type": "/errors/validation-error", "title": "Request Validation Failed", - "status": 400, - "detail": "count must be positive (got: -10)", + "status": 422, + "detail": "Request validation error details", "instance": "/rates/from" } ``` diff --git a/mt5api/__main__.py b/mt5api/__main__.py index 016a43e..6555aaf 100644 --- a/mt5api/__main__.py +++ b/mt5api/__main__.py @@ -11,11 +11,7 @@ get_configured_api_log_level, get_configured_api_port, ) -from .constants import ( - API_APP_IMPORT, - DEFAULT_API_PORT, - MAX_API_PORT, -) +from .constants import API_APP_IMPORT def _get_host() -> str: @@ -33,19 +29,7 @@ def _get_port() -> int: Returns: Port number for the API server. """ - raw_port = get_configured_api_port() - if raw_port is None: - return DEFAULT_API_PORT - - try: - port_value = int(raw_port) - except ValueError: - return DEFAULT_API_PORT - - if not 1 <= port_value <= MAX_API_PORT: - return DEFAULT_API_PORT - - return port_value + return get_configured_api_port() def _get_log_level() -> str: @@ -54,7 +38,7 @@ def _get_log_level() -> str: Returns: Log level string for uvicorn. """ - return get_configured_api_log_level().lower() + return get_configured_api_log_level() def main() -> None: diff --git a/mt5api/config.py b/mt5api/config.py index 43660c7..990c5ee 100644 --- a/mt5api/config.py +++ b/mt5api/config.py @@ -8,6 +8,7 @@ from .constants import ( DEFAULT_API_HOST, DEFAULT_API_LOG_LEVEL, + DEFAULT_API_PORT, DEFAULT_API_ROUTER_PREFIX, DEFAULT_MAX_MARKET_BOOK_SUBSCRIPTIONS, ENV_MT5API_HOST, @@ -16,9 +17,21 @@ ENV_MT5API_PORT, ENV_MT5API_ROUTER_PREFIX, ENV_MT5API_SECRET_KEY, + MAX_API_PORT, ) _VALID_API_ROUTER_PREFIX_PATTERN = re.compile(r"^[A-Za-z0-9_-]+(?:/[A-Za-z0-9_-]+)*$") +_VALID_UVICORN_LOG_LEVELS = {"critical", "error", "warning", "info", "debug", "trace"} +_LOGGING_LOG_LEVEL_BY_API_LEVEL = { + "critical": "CRITICAL", + "error": "ERROR", + "warning": "WARNING", + "info": "INFO", + "debug": "DEBUG", + "trace": "DEBUG", +} +_INVALID_MT5API_PORT_ERROR = "Invalid MT5API_PORT" +_INVALID_MT5API_LOG_LEVEL_ERROR = "Invalid MT5API_LOG_LEVEL" _INVALID_MT5API_ROUTER_PREFIX_ERROR = "Invalid MT5API_ROUTER_PREFIX" _INVALID_MT5API_MAX_MARKET_BOOK_SUBSCRIPTIONS_ERROR = ( "Invalid MT5API_MAX_MARKET_BOOK_SUBSCRIPTIONS" @@ -59,22 +72,52 @@ def get_configured_api_host() -> str: return os.getenv(ENV_MT5API_HOST, DEFAULT_API_HOST) -def get_configured_api_port() -> str | None: - """Get the configured API port string. +def get_configured_api_port() -> int: + """Get the configured API port. Returns: - Raw port string from the environment, or ``None`` if unset. + Port number for the API server. + + Raises: + ValueError: If the configured port is not an integer in the TCP port range. """ - return os.getenv(ENV_MT5API_PORT) + raw_port = os.getenv(ENV_MT5API_PORT) + if raw_port is None: + return DEFAULT_API_PORT + + try: + port = int(raw_port) + except ValueError as error: + raise ValueError(_INVALID_MT5API_PORT_ERROR) from error + + if not 1 <= port <= MAX_API_PORT: + raise ValueError(_INVALID_MT5API_PORT_ERROR) + + return port def get_configured_api_log_level() -> str: """Get the configured API log level. Returns: - Log level string from configuration. + Valid uvicorn log level string from configuration. + + Raises: + ValueError: If the configured log level is unsupported. + """ + log_level = os.getenv(ENV_MT5API_LOG_LEVEL, DEFAULT_API_LOG_LEVEL).strip().lower() + if log_level not in _VALID_UVICORN_LOG_LEVELS: + raise ValueError(_INVALID_MT5API_LOG_LEVEL_ERROR) + return log_level + + +def get_configured_python_log_level() -> str: + """Get a stdlib logging-compatible log level for the API logger. + + Returns: + Python logging level name derived from the configured API log level. """ - return os.getenv(ENV_MT5API_LOG_LEVEL, DEFAULT_API_LOG_LEVEL) + return _LOGGING_LOG_LEVEL_BY_API_LEVEL[get_configured_api_log_level()] def get_configured_api_router_prefix() -> str: diff --git a/mt5api/constants.py b/mt5api/constants.py index e3b74a3..eeeaa4a 100644 --- a/mt5api/constants.py +++ b/mt5api/constants.py @@ -6,7 +6,7 @@ "Provides market data, account information, trading history, calculations, " "and safe operational endpoints via HTTP." ) -API_VERSION = "1.0.0" +API_VERSION = "1.0.1" API_DOCS_URL = "/docs" API_REDOC_URL = "/redoc" API_OPENAPI_URL = "/openapi.json" diff --git a/mt5api/dependencies.py b/mt5api/dependencies.py index efcca74..46436e7 100644 --- a/mt5api/dependencies.py +++ b/mt5api/dependencies.py @@ -80,12 +80,12 @@ def shutdown_mt5_client() -> None: async def replace_mt5_client(config: Mt5Config) -> Mt5DataClient: """Swap the MT5 client singleton with a new connection. - Constructs and initializes the new ``Mt5DataClient`` BEFORE touching the - singleton so a failed reconnect leaves the existing client in place - instead of disconnecting the operator from a working terminal. The new - client is installed atomically; the old client (if any) is shut down on - a best-effort basis after the swap. Callers MUST hold - ``get_mt5_client_lock`` so concurrent reconnect requests do not race. + Constructs and initializes the new ``Mt5DataClient`` before replacing the + singleton reference. The MetaTrader5 Python module has process-global + connection state, so the previous client object must not be shut down after + successful reconnect because that would close the connection just opened by + the new client. Callers MUST hold ``get_mt5_client_lock`` so concurrent + reconnect requests do not race. The raised ``RuntimeError`` deliberately omits the underlying exception text from its message because third-party ``pdmt5``/MetaTrader5 @@ -101,7 +101,7 @@ async def replace_mt5_client(config: Mt5Config) -> Mt5DataClient: Raises: RuntimeError: If the new MT5 client cannot be constructed or - initialized. The previously installed client is preserved. + initialized. """ global _mt5_client # noqa: PLW0603 @@ -113,15 +113,8 @@ async def replace_mt5_client(config: Mt5Config) -> Mt5DataClient: error_message = "Failed to initialize MT5 client" raise RuntimeError(error_message) from e - old_client = _mt5_client _mt5_client = new_client - if old_client is not None: - try: - await asyncio.to_thread(old_client.shutdown) - except Exception: - logger.exception("Failed to shutdown previous MT5 client") - return new_client diff --git a/mt5api/main.py b/mt5api/main.py index 8a86c3d..8252bb9 100644 --- a/mt5api/main.py +++ b/mt5api/main.py @@ -13,9 +13,9 @@ from .auth import is_auth_enabled from .config import ( - get_configured_api_log_level, get_configured_api_router_prefix, get_configured_max_market_book_subscriptions, + get_configured_python_log_level, ) from .constants import ( ACTIVE_MARKET_BOOK_SUBSCRIPTIONS_STATE_KEY, @@ -31,6 +31,7 @@ ) from .dependencies import release_market_book_subscriptions, shutdown_mt5_client from .middleware import add_middleware +from .models import ErrorResponse from .routers import ( account, calc, @@ -65,7 +66,7 @@ def format(self, record: logging.LogRecord) -> str: def _configure_logging() -> None: """Configure structured logging for the API.""" - log_level = get_configured_api_log_level().upper() + log_level = get_configured_python_log_level() handler = logging.StreamHandler() handler.setFormatter(_JsonFormatter()) @@ -96,6 +97,37 @@ def _strip_auth_from_openapi(openapi_schema: dict[str, Any]) -> None: operation.pop("security", None) +def _patch_validation_error_responses(openapi_schema: dict[str, Any]) -> None: + """Advertise RFC 7807 Problem Details for request validation failures.""" + components = openapi_schema.setdefault("components", {}) + if not isinstance(components, dict): + return + + schemas = components.setdefault("schemas", {}) + if isinstance(schemas, dict): + schemas["ErrorResponse"] = ErrorResponse.model_json_schema( + ref_template="#/components/schemas/{model}", + ) + schemas.pop("HTTPValidationError", None) + schemas.pop("ValidationError", None) + + error_response = { + "description": "Validation Error", + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/ErrorResponse"}, + }, + }, + } + + for methods in openapi_schema.get("paths", {}).values(): + if not isinstance(methods, dict): + continue + for operation in methods.values(): + if isinstance(operation, dict) and "422" in operation.get("responses", {}): + operation["responses"]["422"] = error_response + + def _build_openapi_schema(app: FastAPI) -> dict[str, Any]: """Build OpenAPI schema for the current authentication mode. @@ -113,6 +145,7 @@ def _build_openapi_schema(app: FastAPI) -> dict[str, Any]: ) if not is_auth_enabled(): _strip_auth_from_openapi(openapi_schema) + _patch_validation_error_responses(openapi_schema) app.openapi_schema = openapi_schema return openapi_schema diff --git a/mt5api/middleware.py b/mt5api/middleware.py index 3342684..599ce41 100644 --- a/mt5api/middleware.py +++ b/mt5api/middleware.py @@ -4,9 +4,10 @@ import logging import time -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any -from fastapi import Request, status +from fastapi import HTTPException, Request, status +from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse from pdmt5.mt5 import Mt5RuntimeError from pydantic import ValidationError @@ -14,6 +15,8 @@ from .models import ErrorResponse if TYPE_CHECKING: + from collections.abc import Sequence + from fastapi import FastAPI from starlette.middleware.base import RequestResponseEndpoint from starlette.responses import Response @@ -21,6 +24,36 @@ logger = logging.getLogger(__name__) +def _format_validation_error_location(location: Sequence[Any]) -> str: + """Format a Pydantic error location for display. + + Args: + location: Pydantic validation error location tuple. + + Returns: + Human-readable field path without the request-body prefix. + """ + parts = [str(part) for part in location if str(part) != "body"] + return ".".join(parts) if parts else "request" + + +def _format_request_validation_detail(exc: RequestValidationError) -> str: + """Build a sanitized validation detail string without echoing request input. + + Args: + exc: FastAPI request validation error. + + Returns: + Validation summary containing field paths and messages only. + """ + details: list[str] = [] + for error in exc.errors(): + location = _format_validation_error_location(error.get("loc", ())) + message = str(error.get("msg", "Invalid value")) + details.append(f"{location}: {message}") + return "; ".join(details) if details else "Request validation failed" + + def _create_error_response( error_type: str, title: str, @@ -162,11 +195,58 @@ async def logging_middleware( return response +def http_exception_handler( + request: Request, + exc: HTTPException, +) -> JSONResponse: + """Convert FastAPI HTTP exceptions to flat RFC 7807 responses. + + Returns: + Problem Details JSON response. + """ + if isinstance(exc.detail, dict): + detail = exc.detail.get("detail", str(exc.detail)) + error_type = str(exc.detail.get("type", "/errors/http-error")) + title = str(exc.detail.get("title", "HTTP Error")) + else: + detail = str(exc.detail) + error_type = "/errors/http-error" + title = "HTTP Error" + + return _create_error_response( + error_type, + title, + exc.status_code, + detail, + str(request.url), + ) + + +def request_validation_exception_handler( + request: Request, + exc: RequestValidationError, +) -> JSONResponse: + """Convert FastAPI request validation failures to RFC 7807 responses. + + Returns: + Problem Details JSON response. + """ + return _create_error_response( + "/errors/validation-error", + "Request Validation Failed", + status.HTTP_422_UNPROCESSABLE_ENTITY, + _format_request_validation_detail(exc), + str(request.url), + ) + + def add_middleware(app: FastAPI) -> None: """Add middleware and error handlers to the FastAPI application. Args: app: FastAPI application instance. """ + app.exception_handler(HTTPException)(http_exception_handler) + app.exception_handler(RequestValidationError)(request_validation_exception_handler) app.middleware("http")(error_handler_middleware) app.middleware("http")(logging_middleware) diff --git a/mt5api/routers/connection.py b/mt5api/routers/connection.py index 5e782ee..f2cb792 100644 --- a/mt5api/routers/connection.py +++ b/mt5api/routers/connection.py @@ -29,8 +29,8 @@ summary="Login to MT5 terminal", description=( "Reconnect the MT5 terminal using the supplied credentials. The current " - "MT5 client (if any) is shut down and any active market-book " - "subscriptions are released before the new connection is established." + "MT5 singleton is replaced after the new connection is established, then " + "any active market-book subscription tracking is cleared." ), ) async def post_connection_login( @@ -56,8 +56,8 @@ async def post_connection_login( ) async with get_mt5_client_lock(): - await release_market_book_subscriptions(app_request.app) await replace_mt5_client(config) + await release_market_book_subscriptions(app_request.app) logger.info( "MT5 client reconnected (login=%d, server=%s)", diff --git a/mt5api/routers/health.py b/mt5api/routers/health.py index d8c512b..b54647a 100644 --- a/mt5api/routers/health.py +++ b/mt5api/routers/health.py @@ -46,12 +46,12 @@ async def get_health() -> HealthResponse: mt5_version = None try: - client = get_mt5_client() - mt5_connected = True + client = await run_in_threadpool(get_mt5_client) version_dict = await run_in_threadpool(client.version_as_dict) if version_dict: - mt5_version = f"{version_dict.get('version', 'unknown')}" - except RuntimeError as e: + mt5_version = f"{version_dict.get('mt5_terminal_version', 'unknown')}" + mt5_connected = True + except Exception as e: # noqa: BLE001 logger.debug("MT5 not available for health check: %s", e) return HealthResponse( diff --git a/pyproject.toml b/pyproject.toml index b19bbb0..91aae4d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mt5api" -version = "1.0.0" +version = "1.0.1" description = "MetaTrader 5 REST API" authors = [{name = "dceoy", email = "dceoy@users.noreply.github.com"}] maintainers = [{name = "dceoy", email = "dceoy@users.noreply.github.com"}] @@ -9,7 +9,7 @@ license-files = ["LICENSE"] readme = "README.md" requires-python = ">= 3.11, < 3.14" dependencies = [ - "pdmt5 >= 1.0.1", + "pdmt5 >= 1.1.0", "fastapi >= 0.115.0", "uvicorn[standard] >= 0.32.0", "pyarrow >= 18.0.0", diff --git a/tests/conftest.py b/tests/conftest.py index f39466f..5643691 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -44,9 +44,9 @@ def mock_mt5_client() -> Mock: # Mock version method client.version_as_dict.return_value = { - "version": "5.0.4321", + "mt5_terminal_version": "5.0.4321", "build": 4321, - "release_date": "2024-01-01", + "build_release_date": "2024-01-01", } # Mock account info diff --git a/tests/test_auth.py b/tests/test_auth.py index 176536a..f9a0e2b 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -22,7 +22,7 @@ def test_version_endpoint_requires_authentication(client: TestClient) -> None: assert response.status_code == 401 - data = response.json()["detail"] + data = response.json() assert data["type"] == "/errors/unauthorized" assert data["title"] == "Authentication Required" assert "Missing API key" in data["detail"] @@ -35,7 +35,7 @@ def test_version_endpoint_rejects_invalid_api_key(client: TestClient) -> None: assert response.status_code == 401 - data = response.json()["detail"] + data = response.json() assert data["type"] == "/errors/unauthorized" assert data["title"] == "Authentication Failed" assert "Invalid API key" in data["detail"] @@ -61,7 +61,7 @@ def test_symbols_endpoint_requires_authentication( assert response.status_code == 401 mock_mt5_client.symbols_get_as_df.assert_not_called() - data = response.json()["detail"] + data = response.json() assert data["type"] == "/errors/unauthorized" assert data["title"] == "Authentication Required" assert "Missing API key" in data["detail"] @@ -78,7 +78,7 @@ def test_symbols_endpoint_rejects_invalid_api_key( assert response.status_code == 401 mock_mt5_client.symbols_get_as_df.assert_not_called() - data = response.json()["detail"] + data = response.json() assert data["type"] == "/errors/unauthorized" assert data["title"] == "Authentication Failed" assert "Invalid API key" in data["detail"] @@ -120,7 +120,7 @@ def test_trading_endpoints_require_authentication( assert response.status_code == 401 getattr(mock_mt5_client, mock_attr).assert_not_called() - data = response.json()["detail"] + data = response.json() assert data["type"] == "/errors/unauthorized" assert data["title"] == "Authentication Required" assert "Missing API key" in data["detail"] diff --git a/tests/test_cli.py b/tests/test_cli.py index 7f0589b..b953e1e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,8 +4,8 @@ import runpy import sys -from typing import TYPE_CHECKING +import pytest import uvicorn from mt5api.constants import ( @@ -16,9 +16,6 @@ ENV_MT5API_PORT, ) -if TYPE_CHECKING: - import pytest - def test_main_uses_defaults(monkeypatch: pytest.MonkeyPatch) -> None: """main() should use default host/port/log level.""" @@ -71,29 +68,29 @@ def fake_run(*_args: object, **kwargs: object) -> None: assert captured_kwargs["log_level"] == "warning" -def test_main_handles_invalid_port(monkeypatch: pytest.MonkeyPatch) -> None: - """main() should fall back to default port on invalid value.""" - monkeypatch.setenv(ENV_MT5API_PORT, "not-a-number") - - captured_kwargs: dict[str, object] | None = None - - def fake_run(*_args: object, **kwargs: object) -> None: - nonlocal captured_kwargs - captured_kwargs = kwargs +@pytest.mark.parametrize( + "raw_port", + [ + pytest.param("not-a-number", id="non-numeric"), + pytest.param("70000", id="out-of-range"), + ], +) +def test_main_rejects_invalid_port( + monkeypatch: pytest.MonkeyPatch, + raw_port: str, +) -> None: + """main() should fail fast on invalid port values.""" + monkeypatch.setenv(ENV_MT5API_PORT, raw_port) from mt5api import __main__ as api_main # noqa: PLC0415 - monkeypatch.setattr(api_main.uvicorn, "run", fake_run) + with pytest.raises(ValueError, match="Invalid MT5API_PORT"): + api_main.main() - api_main.main() - assert captured_kwargs is not None - assert captured_kwargs["port"] == 8000 - - -def test_main_handles_out_of_range_port(monkeypatch: pytest.MonkeyPatch) -> None: - """main() should fall back to default port on out-of-range value.""" - monkeypatch.setenv(ENV_MT5API_PORT, "70000") +def test_main_accepts_trace_log_level(monkeypatch: pytest.MonkeyPatch) -> None: + """main() should pass uvicorn-supported trace logging through.""" + monkeypatch.setenv(ENV_MT5API_LOG_LEVEL, "trace") captured_kwargs: dict[str, object] | None = None @@ -108,7 +105,7 @@ def fake_run(*_args: object, **kwargs: object) -> None: api_main.main() assert captured_kwargs is not None - assert captured_kwargs["port"] == 8000 + assert captured_kwargs["log_level"] == "trace" def test_module_entrypoint_invokes_main(monkeypatch: pytest.MonkeyPatch) -> None: diff --git a/tests/test_config.py b/tests/test_config.py index 67ef770..172a9c2 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -8,7 +8,9 @@ from mt5api.constants import ( DEFAULT_MAX_MARKET_BOOK_SUBSCRIPTIONS, + ENV_MT5API_LOG_LEVEL, ENV_MT5API_MAX_MARKET_BOOK_SUBSCRIPTIONS, + ENV_MT5API_PORT, ENV_MT5API_ROUTER_PREFIX, ENV_MT5API_SECRET_KEY, ) @@ -72,6 +74,82 @@ def test_get_configured_mt5api_secret_key( assert config.get_configured_mt5api_secret_key() == expected_secret_key +@pytest.mark.parametrize( + ("raw_port", "expected_port"), + [ + (None, 8000), + ("1", 1), + ("65535", 65535), + ], +) +def test_get_configured_api_port( + monkeypatch: pytest.MonkeyPatch, + raw_port: str | None, + expected_port: int, +) -> None: + """API port config should read a valid TCP port.""" + from mt5api import config # noqa: PLC0415 + + if raw_port is None: + monkeypatch.delenv(ENV_MT5API_PORT, raising=False) + else: + monkeypatch.setenv(ENV_MT5API_PORT, raw_port) + + assert config.get_configured_api_port() == expected_port + + +@pytest.mark.parametrize("raw_port", ["0", "65536", "not-a-number"]) +def test_get_configured_api_port_rejects_invalid_values( + monkeypatch: pytest.MonkeyPatch, + raw_port: str, +) -> None: + """API port config should reject invalid values.""" + from mt5api import config # noqa: PLC0415 + + monkeypatch.setenv(ENV_MT5API_PORT, raw_port) + + with pytest.raises(ValueError, match="Invalid MT5API_PORT"): + config.get_configured_api_port() + + +@pytest.mark.parametrize( + ("raw_log_level", "expected_log_level", "expected_python_log_level"), + [ + (None, "info", "INFO"), + ("WARNING", "warning", "WARNING"), + ("trace", "trace", "DEBUG"), + ], +) +def test_get_configured_api_log_level( + monkeypatch: pytest.MonkeyPatch, + raw_log_level: str | None, + expected_log_level: str, + expected_python_log_level: str, +) -> None: + """API log-level config should support uvicorn levels.""" + from mt5api import config # noqa: PLC0415 + + if raw_log_level is None: + monkeypatch.delenv(ENV_MT5API_LOG_LEVEL, raising=False) + else: + monkeypatch.setenv(ENV_MT5API_LOG_LEVEL, raw_log_level) + + assert config.get_configured_api_log_level() == expected_log_level + assert config.get_configured_python_log_level() == expected_python_log_level + + +def test_get_configured_api_log_level_rejects_invalid_values( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """API log-level config should reject unsupported values.""" + from mt5api import config # noqa: PLC0415 + + monkeypatch.setenv(ENV_MT5API_LOG_LEVEL, "verbose") + + with pytest.raises(ValueError, match="Invalid MT5API_LOG_LEVEL"): + config.get_configured_api_log_level() + + @pytest.mark.parametrize( ("raw_limit", "expected_limit"), [ diff --git a/tests/test_connection.py b/tests/test_connection.py index 663c84c..816ac2d 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -95,15 +95,15 @@ def test_post_connection_login_reconnects( assert config.login == 12345 assert config.server == "MetaQuotes-Demo" assert config.timeout == 60000 - assert config.password == "s3cret" # noqa: S105 + assert config.password.get_secret_value() == "s3cret" assert all("s3cret" not in record.getMessage() for record in caplog.records) -def test_post_connection_login_releases_market_book( +def test_post_connection_login_releases_market_book_after_reconnect( connection_client: tuple[TestClient, dict[str, Any]], mocker: MockerFixture, ) -> None: - """Reconnect should release tracked market-book subscriptions first.""" + """Reconnect should release tracked market-book subscriptions after success.""" test_client, _ = connection_client cleanup_client = mocker.Mock(name="cleanup_client") app.state.active_market_book_subscriptions = {"EURUSD", "GBPUSD"} @@ -127,6 +127,43 @@ def test_post_connection_login_releases_market_book( assert app.state.market_book_cleanup_client is None +def test_post_connection_login_preserves_market_book_on_reconnect_failure( + connection_client: tuple[TestClient, dict[str, Any]], + mocker: MockerFixture, +) -> None: + """Reconnect failure should not drop tracked market-book subscriptions.""" + test_client, recorder = connection_client + cleanup_client = mocker.Mock(name="cleanup_client") + app.state.active_market_book_subscriptions = {"EURUSD"} + app.state.market_book_cleanup_client = cleanup_client + + async def fail_replace(config: object) -> None: + await asyncio.sleep(0) + recorder["configs"].append(config) + error_message = "connection unavailable" + raise RuntimeError(error_message) + + mocker.patch( + "mt5api.routers.connection.replace_mt5_client", + side_effect=fail_replace, + ) + + response = test_client.post( + "/connection/login", + json={ + "login": 7, + "password": "p", + "server": "Demo", + }, + headers={API_KEY_HEADER_NAME: "test-api-key-12345"}, + ) + + assert response.status_code == 503 + cleanup_client.market_book_release.assert_not_called() + assert app.state.active_market_book_subscriptions == {"EURUSD"} + assert app.state.market_book_cleanup_client is cleanup_client + + def test_post_connection_login_requires_api_key( connection_client: tuple[TestClient, dict[str, Any]], ) -> None: @@ -208,6 +245,24 @@ def test_post_connection_login_validates_fields( assert recorder["configs"] == [] +def test_post_connection_login_validation_does_not_leak_password( + connection_client: tuple[TestClient, dict[str, Any]], +) -> None: + """Invalid login payloads must not echo submitted passwords in 422 bodies.""" + test_client, recorder = connection_client + password = "very-secret-pa55word-XYZ" # noqa: S105 + + response = test_client.post( + "/connection/login", + json={"login": 1, "password": password * 6, "server": "S"}, + headers={API_KEY_HEADER_NAME: "test-api-key-12345"}, + ) + + assert response.status_code == 422 + assert password not in response.text + assert recorder["configs"] == [] + + def test_post_connection_login_does_not_leak_password_on_failure( mocker: MockerFixture, ) -> None: @@ -345,7 +400,7 @@ async def run_requests() -> list[httpx.Response]: def test_replace_mt5_client_swaps_singleton( mocker: MockerFixture, ) -> None: - """replace_mt5_client shuts the old client down and installs the new one.""" + """replace_mt5_client installs the new client without shutting down MT5.""" class DummyClient: def __init__(self, config: object) -> None: @@ -374,7 +429,7 @@ async def run() -> DummyClient: assert new_client.config is config assert new_client.initialized is True - old_client.shutdown.assert_called_once_with() + old_client.shutdown.assert_not_called() assert dependencies._mt5_client is new_client # pyright: ignore[reportPrivateUsage] @@ -437,31 +492,3 @@ async def run() -> DummyClient: new_client = asyncio.run(run()) assert dependencies._mt5_client is new_client # pyright: ignore[reportPrivateUsage] - - -def test_replace_mt5_client_continues_when_old_shutdown_fails( - mocker: MockerFixture, -) -> None: - """A failing old-client shutdown should not block the new connection.""" - - class DummyClient: - def __init__(self, config: object) -> None: - self.config = config - - def initialize_and_login_mt5(self) -> None: - return None - - old_client = mocker.Mock(name="old_client") - old_client.shutdown.side_effect = RuntimeError("cannot shut down") - mocker.patch.object(dependencies, "_mt5_client", old_client) - mocker.patch.object(dependencies, "Mt5DataClient", DummyClient) - - async def run() -> DummyClient: - async with dependencies.get_mt5_client_lock(): - client = await dependencies.replace_mt5_client(mocker.Mock(name="config")) - assert isinstance(client, DummyClient) - return client - - new_client = asyncio.run(run()) - - assert dependencies._mt5_client is new_client # pyright: ignore[reportPrivateUsage] diff --git a/tests/test_health.py b/tests/test_health.py index 0c7b60e..2d5d362 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -49,9 +49,9 @@ def test_version_endpoint_returns_mt5_version( assert payload["count"] == 1 data = payload["data"] - assert data["version"] == "5.0.4321" + assert data["mt5_terminal_version"] == "5.0.4321" assert data["build"] == 4321 - assert "release_date" in data + assert "build_release_date" in data def test_version_endpoint_returns_parquet( @@ -73,7 +73,18 @@ def test_docs_and_openapi_available(client: TestClient) -> None: openapi_response = client.get("/openapi.json") assert openapi_response.status_code == 200 - assert "paths" in openapi_response.json() + openapi = openapi_response.json() + assert "paths" in openapi + assert "ErrorResponse" in openapi["components"]["schemas"] + assert "HTTPValidationError" not in openapi["components"]["schemas"] + assert openapi["paths"]["/rates/from"]["get"]["responses"]["422"] == { + "description": "Validation Error", + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/ErrorResponse"}, + }, + }, + } def test_last_error_returns_json( @@ -124,6 +135,55 @@ def raise_runtime_error() -> None: response = await health.get_health() assert response.mt5_connected is False + assert response.status == "unhealthy" + + +@pytest.mark.asyncio +async def test_get_health_handles_version_probe_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test get_health reports unhealthy when MT5 version probing fails.""" + from mt5api.routers import health # noqa: PLC0415 + + class DummyClient: + def version_as_dict(self) -> dict[str, str]: + error_message = "MT5 disconnected" + raise TypeError(error_message) + + def get_client() -> DummyClient: + return DummyClient() + + monkeypatch.setattr(health, "get_mt5_client", get_client) + + response = await health.get_health() + + assert response.mt5_connected is False + assert response.status == "unhealthy" + assert response.mt5_version is None + + +@pytest.mark.asyncio +async def test_get_health_handles_unexpected_probe_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test get_health reports unhealthy for unexpected MT5 probe failures.""" + from mt5api.routers import health # noqa: PLC0415 + + class DummyClient: + def version_as_dict(self) -> dict[str, str]: + error_message = "IPC failure" + raise OSError(error_message) + + def get_client() -> DummyClient: + return DummyClient() + + monkeypatch.setattr(health, "get_mt5_client", get_client) + + response = await health.get_health() + + assert response.mt5_connected is False + assert response.status == "unhealthy" + assert response.mt5_version is None @pytest.mark.asyncio diff --git a/tests/test_main.py b/tests/test_main.py index 4ce4985..e497a22 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -66,6 +66,108 @@ def test_strip_auth_from_openapi_preserves_other_security_schemes() -> None: assert openapi_schema["components"]["securitySchemes"] == {"Other": {}} +def test_patch_validation_error_responses_updates_422_schema() -> None: + """OpenAPI validation responses should advertise RFC 7807 Problem Details.""" + from mt5api import main # noqa: PLC0415 + + openapi_schema: dict[str, Any] = { + "components": { + "schemas": { + "HTTPValidationError": {"type": "object"}, + "ValidationError": {"type": "object"}, + }, + }, + "paths": { + "/bad": [], + "/rates/from": { + "get": { + "responses": { + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": ( + "#/components/schemas/HTTPValidationError" + ), + }, + }, + }, + }, + }, + }, + }, + }, + } + + main._patch_validation_error_responses(openapi_schema) # pyright: ignore[reportPrivateUsage] + + schemas = openapi_schema["components"]["schemas"] + assert "ErrorResponse" in schemas + assert "HTTPValidationError" not in schemas + assert "ValidationError" not in schemas + assert openapi_schema["paths"]["/rates/from"]["get"]["responses"]["422"] == { + "description": "Validation Error", + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/ErrorResponse"}, + }, + }, + } + + +def test_patch_validation_error_responses_handles_non_dict_components() -> None: + """OpenAPI validation patching should tolerate missing component mappings.""" + from mt5api import main # noqa: PLC0415 + + openapi_schema: dict[str, Any] = {"components": None, "paths": {}} + + main._patch_validation_error_responses(openapi_schema) # pyright: ignore[reportPrivateUsage] + + assert openapi_schema["components"] is None + + +def test_patch_validation_error_responses_handles_non_dict_schemas() -> None: + """OpenAPI validation patching should still update 422 responses.""" + from mt5api import main # noqa: PLC0415 + + openapi_schema: dict[str, Any] = { + "components": {"schemas": []}, + "paths": { + "/rates/from": { + "get": { + "responses": { + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": ( + "#/components/schemas/HTTPValidationError" + ), + }, + }, + }, + }, + }, + }, + }, + }, + } + + main._patch_validation_error_responses(openapi_schema) # pyright: ignore[reportPrivateUsage] + + assert openapi_schema["components"]["schemas"] == [] + assert openapi_schema["paths"]["/rates/from"]["get"]["responses"]["422"] == { + "description": "Validation Error", + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/ErrorResponse"}, + }, + }, + } + + def test_release_market_book_subscriptions_clears_state( mocker: MockerFixture, ) -> None: diff --git a/tests/test_market.py b/tests/test_market.py index c8d2c95..3c31121 100644 --- a/tests/test_market.py +++ b/tests/test_market.py @@ -223,7 +223,9 @@ def test_get_rates_from_rejects_invalid_mt5_timeframe( ) assert response.status_code == 422 - assert response.json()["detail"][0]["loc"][-1] == "timeframe" + payload = response.json() + assert payload["type"] == "/errors/validation-error" + assert "timeframe" in payload["detail"] def test_get_rates_from_rejects_invalid_mt5_timeframe_name( @@ -243,7 +245,9 @@ def test_get_rates_from_rejects_invalid_mt5_timeframe_name( ) assert response.status_code == 422 - assert response.json()["detail"][0]["loc"][-1] == "timeframe" + payload = response.json() + assert payload["type"] == "/errors/validation-error" + assert "timeframe" in payload["detail"] def test_openapi_documents_mt5_timeframes_consistently( @@ -419,7 +423,9 @@ def test_get_ticks_from_rejects_invalid_mt5_copy_ticks_name( ) assert response.status_code == 422 - assert response.json()["detail"][0]["loc"][-1] == "flags" + payload = response.json() + assert payload["type"] == "/errors/validation-error" + assert "flags" in payload["detail"] def test_openapi_documents_mt5_copy_ticks_consistently( diff --git a/tests/test_middleware.py b/tests/test_middleware.py index dd0fc89..52cd641 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -6,12 +6,18 @@ import json from typing import TYPE_CHECKING, cast -from fastapi import FastAPI +import pytest +from fastapi import FastAPI, HTTPException +from fastapi.exceptions import RequestValidationError from fastapi.testclient import TestClient from pdmt5.mt5 import Mt5RuntimeError from pydantic import BaseModel -from mt5api.middleware import _create_error_response, add_middleware +from mt5api.middleware import ( + _create_error_response, + _format_request_validation_detail, + add_middleware, +) if TYPE_CHECKING: from collections.abc import Callable @@ -45,21 +51,57 @@ def test_create_error_response_builds_problem_details() -> None: } -def test_error_handler_handles_mt5_runtime_error() -> None: - """Test Mt5RuntimeError mapping to 503 response.""" +class _DummyMt5Error(Mt5RuntimeError): + """Test-specific MT5 error.""" - class DummyMt5Error(Mt5RuntimeError): - """Test-specific MT5 error.""" + +class _DummyValueError(ValueError): + """Test-specific value error.""" + + +class _DummyRuntimeError(RuntimeError): + """Test-specific runtime error.""" + + +class _UnexpectedError(Exception): + """Test-specific unexpected error.""" + + +@pytest.mark.parametrize( + ("exception_type", "expected_status", "expected_type"), + [ + pytest.param(_DummyMt5Error, 503, "/errors/mt5-error", id="mt5-runtime-error"), + pytest.param(_DummyValueError, 400, "/errors/invalid-input", id="value-error"), + pytest.param( + _DummyRuntimeError, + 503, + "/errors/runtime-error", + id="runtime-error", + ), + pytest.param( + _UnexpectedError, + 500, + "/errors/internal-error", + id="unexpected-error", + ), + ], +) +def test_error_handler_maps_exceptions( + exception_type: type[BaseException], + expected_status: int, + expected_type: str, +) -> None: + """Test exception types map to RFC 7807 error responses.""" def handler() -> None: - raise DummyMt5Error + raise exception_type() client = TestClient(_create_app(handler)) response = client.get("/boom") assert (response.status_code, response.json()["type"]) == ( - 503, - "/errors/mt5-error", + expected_status, + expected_type, ) @@ -81,58 +123,90 @@ def handler() -> None: ) -def test_error_handler_handles_value_error() -> None: - """Test ValueError mapping to 400 response.""" - - class DummyValueError(ValueError): - """Test-specific value error.""" +def test_http_exception_handler_flattens_problem_details() -> None: + """HTTPException details should not be nested under a detail key.""" def handler() -> None: - raise DummyValueError + raise HTTPException( + status_code=401, + detail={ + "type": "/errors/unauthorized", + "title": "Authentication Required", + "detail": "Missing API key.", + }, + ) client = TestClient(_create_app(handler)) response = client.get("/boom") - assert (response.status_code, response.json()["type"]) == ( - 400, - "/errors/invalid-input", - ) - + assert response.status_code == 401 + assert response.json() == { + "type": "/errors/unauthorized", + "title": "Authentication Required", + "status": 401, + "detail": "Missing API key.", + "instance": "http://testserver/boom", + } -def test_error_handler_handles_runtime_error() -> None: - """Test RuntimeError mapping to 503 response.""" - class DummyRuntimeError(RuntimeError): - """Test-specific runtime error.""" +def test_http_exception_handler_handles_plain_detail() -> None: + """HTTPException string details should become generic problem details.""" def handler() -> None: - raise DummyRuntimeError + raise HTTPException(status_code=404, detail="Missing") client = TestClient(_create_app(handler)) response = client.get("/boom") - assert (response.status_code, response.json()["type"]) == ( - 503, - "/errors/runtime-error", - ) + assert response.status_code == 404 + assert response.json() == { + "type": "/errors/http-error", + "title": "HTTP Error", + "status": 404, + "detail": "Missing", + "instance": "http://testserver/boom", + } -def test_error_handler_handles_unexpected_error() -> None: - """Test unexpected error mapping to 500 response.""" +def test_request_validation_error_returns_problem_details() -> None: + """FastAPI request validation should use RFC 7807 problem details.""" + app = FastAPI() + add_middleware(app) - class UnexpectedError(Exception): - """Test-specific unexpected error.""" + def handler(item_id: int) -> dict[str, int]: + return {"item_id": item_id} - def handler() -> None: - raise UnexpectedError + app.get("/items/{item_id}")(handler) - client = TestClient(_create_app(handler)) - response = client.get("/boom") + client = TestClient(app) + response = client.get("/items/not-an-int") - assert (response.status_code, response.json()["type"]) == ( - 500, - "/errors/internal-error", + assert response.status_code == 422 + payload = response.json() + assert payload["type"] == "/errors/validation-error" + assert payload["title"] == "Request Validation Failed" + assert payload["status"] == 422 + assert "item_id" in payload["detail"] + assert payload["instance"] == "http://testserver/items/not-an-int" + + +def test_format_request_validation_detail_omits_request_input() -> None: + """Validation detail should summarize field errors without echoing input.""" + exc = RequestValidationError([ + { + "type": "too_long", + "loc": ("body", "password"), + "msg": "Value should have at most 128 items after validation, not 129", + "input": "super-secret-password-value", + } + ]) + + detail = _format_request_validation_detail(exc) + + assert detail == ( + "password: Value should have at most 128 items after validation, not 129" ) + assert "super-secret-password-value" not in detail def test_logging_middleware_adds_process_time_header() -> None: diff --git a/tests/test_trading.py b/tests/test_trading.py index b29e682..b1b0ab5 100644 --- a/tests/test_trading.py +++ b/tests/test_trading.py @@ -98,7 +98,9 @@ def test_post_order_check_validates_payload( ) assert response.status_code == 422 - assert response.json()["detail"][0]["loc"][-1] == "symbol" + payload = response.json() + assert payload["type"] == "/errors/validation-error" + assert "symbol" in payload["detail"] mock_mt5_client.order_check_as_dict.assert_not_called() diff --git a/uv.lock b/uv.lock index 8628ea3..0510028 100644 --- a/uv.lock +++ b/uv.lock @@ -427,7 +427,7 @@ name = "metatrader5" version = "5.0.5735" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "numpy", marker = "sys_platform == 'win32'" }, + { name = "numpy" }, ] wheels = [ { url = "https://files.pythonhosted.org/packages/07/0b/3dd5143f14319dca393a3bcf11ddf24f36f16dfb8c6d1bf2b193ce0c5733/metatrader5-5.0.5735-cp311-cp311-win_amd64.whl", hash = "sha256:0d05b69cef5eb3f43ea47cb6d36dac0e7e15f82a4fb77974439c565daa54d1c9", size = 48091, upload-time = "2026-04-04T16:44:08.677Z" }, @@ -556,7 +556,7 @@ wheels = [ [[package]] name = "mt5api" -version = "1.0.0" +version = "1.0.1" source = { editable = "." } dependencies = [ { name = "fastapi" }, @@ -588,7 +588,7 @@ dev = [ requires-dist = [ { name = "fastapi", specifier = ">=0.115.0" }, { name = "httpx", specifier = ">=0.27.0" }, - { name = "pdmt5", specifier = ">=1.0.1" }, + { name = "pdmt5", specifier = ">=1.1.0" }, { name = "pyarrow", specifier = ">=18.0.0" }, { name = "python-multipart", specifier = ">=0.0.31" }, { name = "uvicorn", extras = ["standard"], specifier = ">=0.32.0" }, @@ -764,16 +764,16 @@ wheels = [ [[package]] name = "pdmt5" -version = "1.0.1" +version = "1.1.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "metatrader5", marker = "sys_platform == 'win32'" }, { name = "pandas" }, { name = "pydantic" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/e0/9d/55f22e0155ad36e82ecd4753fbad822966844edf0597a49e81a0dfdc883a/pdmt5-1.0.1.tar.gz", hash = "sha256:1ceb6637f4976cb53903f85584d09d7e4e870b44301d7e720d8ff0d42484f3fc", size = 114846, upload-time = "2026-06-26T16:46:11.733Z" } +sdist = { url = "https://files.pythonhosted.org/packages/35/7a/7caafb93b74ccaa60a1c844948698bed714e04aa3be292a05fd71864faa0/pdmt5-1.1.0.tar.gz", hash = "sha256:fcdab1924e001204ae776dd2afc1f150fe01e6944cba99dae72af23bc99137c6", size = 22160, upload-time = "2026-07-04T04:41:51.476Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/d7/6f/2f4a400a5d3373f4d722516c914d45587aa2b3369870225c99242d6472e3/pdmt5-1.0.1-py3-none-any.whl", hash = "sha256:a54df6d2469781a5959e1f50e9911a492cde17ee1e2c2d9b6d84f84a7bf7c7e7", size = 20059, upload-time = "2026-06-26T16:46:10.432Z" }, + { url = "https://files.pythonhosted.org/packages/6f/80/1062ea4e4c82de81d91ba8bb43b1a6d4903771010d87eca15d5d18d827df/pdmt5-1.1.0-py3-none-any.whl", hash = "sha256:603273065814824680d6ec8847139b3029c7dd04da7c25868ad7f4ede55a9acf", size = 20489, upload-time = "2026-07-04T04:41:49.999Z" }, ] [[package]]