diff --git a/README.md b/README.md index 3052204..b09f281 100644 --- a/README.md +++ b/README.md @@ -92,16 +92,21 @@ Schema contracts live in `mt5cli.schemas` (`DataKind`, `validate_schema`, `norma Trading applications can depend on `mt5cli` imports only; terminal path, credentials, server, and timeout are forwarded to `pdmt5.Mt5Config`, numeric login strings are coerced to integers, and empty login strings are treated as -unset. +unset. Pass `allow_whole_dollar_env=True` to expand `${ENV_VAR}` and bare +`$ENV_NAME` placeholders in connection string parameters before coercion. ```python from mt5cli import ( + build_config, calculate_spread_ratio, create_trading_client, get_account_snapshot, mt5_trading_session, ) +# Login from environment — numeric string is coerced to int automatically +config = build_config(login="$MT5_LOGIN", allow_whole_dollar_env=True) + with mt5_trading_session( path=r"C:\Program Files\MetaTrader 5\terminal64.exe", login="12345", @@ -248,7 +253,7 @@ rates = collect_latest_closed_rates_by_granularity( eurusd_m1 = rates["EURUSD", "M1"] # closed bars only ``` -- **Credential resolution**: use `resolve_account_spec()` / `resolve_account_specs()` to merge explicit override values over `AccountSpec` fields and expand `${ENV_VAR}` placeholders (via `substitute_env_placeholders()`), raising `ValueError` for missing variables. This keeps secrets out of plan/config files without coupling to any strategy code. +- **Credential resolution**: use `resolve_account_spec()` / `resolve_account_specs()` to merge explicit override values over `AccountSpec` fields and expand `${ENV_VAR}` placeholders (via `substitute_env_placeholders()`), raising `ValueError` for missing variables. This keeps secrets out of plan/config files without coupling to any strategy code. For config dicts or nested structures loaded from YAML/TOML, use `substitute_mapping_values(data, keys={"login", "password"})` to expand placeholders only for caller-specified keys — key names are never hard-coded in mt5cli. - **Throttled history updates**: use `ThrottledHistoryUpdater` to wrap `update_history()` with a minimum `interval_seconds` between successful runs (monotonic clock). Call `should_update()` / `update(client, symbols)` from an application loop; errors propagate by default, or pass `suppress_errors=True` to swallow recoverable `Mt5*Error`, `sqlite3.Error`, `ValueError`, `OSError`, and MT5 client capability errors for history API methods without advancing the throttle (other `AttributeError` / `TypeError` values always propagate). Pass `update_backend` to inject a custom history update callable (same keyword arguments as `update_history`) instead of monkey-patching `mt5cli.sdk.update_history`. - **Trading session helpers**: use `mt5_trading_session()` for a trading-capable `pdmt5.Mt5TradingClient` that initializes/logs in via `Mt5Config.path` and always shuts down safely. Pair with `detect_position_side()`, `calculate_margin_and_volume()`, and `determine_order_limits()` for generic position and sizing utilities. Keep read-only collection on `mt5_session()` / `MT5Client`. - **Granularity-keyed rate loading**: `load_rate_series_by_granularity()` builds targets with `build_rate_targets()`, loads them with `load_rate_series_from_sqlite()`, and returns a mapping keyed by `(symbol | None, granularity_name)` such as `("EURUSD", "M1")` to reduce downstream boilerplate. diff --git a/docs/api/public-contract.md b/docs/api/public-contract.md index d4f7661..a0840a6 100644 --- a/docs/api/public-contract.md +++ b/docs/api/public-contract.md @@ -28,23 +28,25 @@ These names are exported from `mt5cli` and covered by the contract in ### Session lifecycle and configuration -| Symbol | Role | -| ----------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | -| `MT5Client` | Read-only data client with optional `order_check` / `order_send` | -| `build_config` | Build `pdmt5.Mt5Config` from connection fields | -| `mt5_session` | Context manager: initialize, login, yield client, shutdown | -| `create_trading_client`, `mt5_trading_session` | Trading-capable `pdmt5.Mt5TradingClient` lifecycle | -| `AccountSpec` | Generic account group: symbols plus optional credentials | -| `resolve_account_spec`, `resolve_account_specs` | Merge overrides and expand `${ENV_VAR}` placeholders; opt-in `allow_whole_dollar_env` for bare `$NAME` | -| `substitute_env_placeholders` | Replace `${NAME}` substrings from the environment; opt-in `allow_whole_dollar_env` for whole-value `$NAME` | +| Symbol | Role | +| ----------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `MT5Client` | Read-only data client with optional `order_check` / `order_send` | +| `build_config` | Build `pdmt5.Mt5Config` from connection fields; `login` accepts `int \| str \| None` — numeric strings are coerced to `int`, blank strings are treated as unset, and `${ENV_VAR}` / `$ENV_NAME` placeholders in string parameters are expanded when `allow_whole_dollar_env=True` | +| `mt5_session` | Context manager: initialize, login, yield client, shutdown | +| `create_trading_client`, `mt5_trading_session` | Trading-capable `pdmt5.Mt5TradingClient` lifecycle | +| `AccountSpec` | Generic account group: symbols plus optional credentials | +| `resolve_account_spec`, `resolve_account_specs` | Merge overrides and expand `${ENV_VAR}` placeholders; opt-in `allow_whole_dollar_env` for bare `$NAME` | +| `substitute_env_placeholders` | Replace `${NAME}` substrings from the environment; opt-in `allow_whole_dollar_env` for whole-value `$NAME` | +| `substitute_mapping_values` | Recursively traverse a dict/list/scalar structure and substitute `${ENV_VAR}` placeholders for caller-selected mapping keys only; optionally normalise blank strings to `None` for a separate caller-selected key set; does not hard-code any application-specific key names | Credential resolution is generic: any environment variable name may appear inside `${...}`. mt5cli does not hard-code application-specific keys such as `mt5_login` or `mt5_exe`. Pass `allow_whole_dollar_env=True` to `substitute_env_placeholders()`, -`resolve_account_spec()`, `resolve_account_specs()`, and `build_config()` to -additionally expand strings whose entire value is a bare `$ENV_NAME` identifier. +`substitute_mapping_values()`, `resolve_account_spec()`, `resolve_account_specs()`, +and `build_config()` to additionally expand strings whose entire value is a bare +`$ENV_NAME` identifier. Partial strings such as `"plan$pass"`, `"abc$ENV"`, or `"$ENV-suffix"` are **never** expanded — only an exact `$IDENTIFIER` whole-string match qualifies. Default is `False` to preserve backward compatibility. diff --git a/mt5cli/__init__.py b/mt5cli/__init__.py index 8369edf..ce184c6 100644 --- a/mt5cli/__init__.py +++ b/mt5cli/__init__.py @@ -92,6 +92,7 @@ from .sdk import ( resolve_account_spec, resolve_account_specs, substitute_env_placeholders, + substitute_mapping_values, symbol_info, symbol_info_tick, symbols, @@ -283,6 +284,7 @@ __all__ = [ "resolve_rate_view_names", "schema_columns", "substitute_env_placeholders", + "substitute_mapping_values", "symbol_info", "symbol_info_tick", "symbols", diff --git a/mt5cli/contract.py b/mt5cli/contract.py index 494b49e..2915183 100644 --- a/mt5cli/contract.py +++ b/mt5cli/contract.py @@ -77,6 +77,7 @@ STABLE_SDK_EXPORTS: frozenset[str] = frozenset({ "resolve_rate_view_name", "resolve_rate_view_names", "substitute_env_placeholders", + "substitute_mapping_values", "update_history", "update_history_with_config", "update_sltp_for_open_positions", diff --git a/mt5cli/sdk.py b/mt5cli/sdk.py index 9e93377..b048df2 100644 --- a/mt5cli/sdk.py +++ b/mt5cli/sdk.py @@ -40,7 +40,7 @@ from .utils import ( from .utils import coerce_login as _coerce_login if TYPE_CHECKING: - from collections.abc import Callable, Iterator, Sequence + from collections.abc import Callable, Collection, Iterator, Sequence UpdateHistoryBackend = Callable[..., None] @@ -142,6 +142,7 @@ __all__ = [ "resolve_account_spec", "resolve_account_specs", "substitute_env_placeholders", + "substitute_mapping_values", "symbol_info", "symbol_info_tick", "symbols", @@ -305,7 +306,7 @@ def _fetch_minimum_margins(client: Mt5DataClient, symbol: str) -> pd.DataFrame: def build_config( *, path: str | None = None, - login: int | None = None, + login: int | str | None = None, password: str | None = None, server: str | None = None, timeout: int | None = None, @@ -315,14 +316,19 @@ def build_config( Args: path: Optional terminal executable path. - login: Optional trading account login. + login: Optional trading account login. Integers are preserved. String + values are coerced: empty or whitespace-only strings become + ``None``; numeric strings such as ``"12345"`` are converted to + ``int``; non-numeric strings raise ``ValueError``. When + ``allow_whole_dollar_env=True``, ``$ENV_NAME`` and + ``${ENV_NAME}`` placeholders are expanded before coercion. password: Optional trading account password. server: Optional trading server name. timeout: Optional connection timeout in milliseconds. allow_whole_dollar_env: When ``True``, string parameters that are exactly ``$ENV_NAME`` are expanded from the environment. Applies - to ``path``, ``password``, and ``server``. Default ``False`` - preserves existing behavior. + to ``path``, ``login``, ``password``, and ``server``. Default + ``False`` preserves existing behavior. Returns: Configured ``Mt5Config`` instance. @@ -330,6 +336,8 @@ def build_config( if allow_whole_dollar_env: if path is not None: path = substitute_env_placeholders(path, allow_whole_dollar_env=True) + if isinstance(login, str): + login = substitute_env_placeholders(login, allow_whole_dollar_env=True) if password is not None: password = substitute_env_placeholders( password, allow_whole_dollar_env=True @@ -338,7 +346,7 @@ def build_config( server = substitute_env_placeholders(server, allow_whole_dollar_env=True) return Mt5Config( path=path, - login=login, + login=_coerce_login(login), password=password, server=server, timeout=timeout, @@ -1442,6 +1450,75 @@ def substitute_env_placeholders( return "".join(parts) +def substitute_mapping_values( + data: object, + *, + keys: Collection[str], + allow_whole_dollar_env: bool = False, + blank_string_keys_as_none: Collection[str] = (), +) -> object: + """Recursively substitute environment placeholders for selected mapping keys. + + Traverses nested dicts and lists, expanding ``${ENV_VAR}`` (and + ``$ENV_NAME`` when ``allow_whole_dollar_env=True``) in string values + whose immediate parent dict key is in ``keys``. Fields whose key is + not in ``keys`` are preserved exactly, including literal dollar signs. + Strings that are direct elements of a list are never substituted; + substitution only applies to strings that are immediate dict values. + + This is a generic downstream config utility. Key names such as + ``mt5_login`` or ``mt5_password`` must be supplied by the caller; + mt5cli does not hard-code any application-specific key names. + Callers are responsible for ensuring ``data`` has bounded nesting depth; + deeply nested or self-referential structures will hit Python's recursion + limit. + + Args: + data: Arbitrarily nested dict/list/scalar value to process. + keys: Mapping keys whose string values receive placeholder + substitution. + allow_whole_dollar_env: When ``True``, a string that is exactly + ``$ENV_NAME`` (whole value) is also expanded from the + environment in addition to ``${ENV_NAME}`` placeholders. + Default ``False`` expands ``${ENV_NAME}`` only. + blank_string_keys_as_none: Mapping keys for which blank strings + (after any substitution) are normalised to ``None``. A key + may appear in ``blank_string_keys_as_none`` without also + appearing in ``keys``. + + Returns: + The processed value. Dicts and lists are rebuilt into new + containers with selected string values substituted and + blank-normalised. Scalar inputs (non-dict, non-list) are + returned as-is. + """ + keys_set: frozenset[str] = frozenset(keys) + blank_keys_set: frozenset[str] = frozenset(blank_string_keys_as_none) + + def _visit(node: object, current_key: str | None) -> object: + if isinstance(node, dict): + typed = cast("dict[object, object]", node) + return { + k: _visit(v, k if isinstance(k, str) else None) + for k, v in typed.items() + } + if isinstance(node, list): + typed_list = cast("list[object]", node) + return [_visit(item, None) for item in typed_list] + if not isinstance(node, str): + return node + text = node + if current_key in keys_set: + text = substitute_env_placeholders( + node, allow_whole_dollar_env=allow_whole_dollar_env + ) + if current_key in blank_keys_set and not text.strip(): + return None + return text + + return _visit(data, None) + + def _resolve_field( override: str | None, account_value: str | None, diff --git a/pyproject.toml b/pyproject.toml index 6f75b25..b305dc8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mt5cli" -version = "0.9.4" +version = "0.9.5" description = "Generic MT5 data and execution infrastructure for Python applications" authors = [{name = "dceoy", email = "dceoy@users.noreply.github.com"}] maintainers = [{name = "dceoy", email = "dceoy@users.noreply.github.com"}] diff --git a/tests/test_sdk.py b/tests/test_sdk.py index 60e831f..aeebb2b 100644 --- a/tests/test_sdk.py +++ b/tests/test_sdk.py @@ -54,6 +54,7 @@ from mt5cli.sdk import ( resolve_account_spec, resolve_account_specs, substitute_env_placeholders, + substitute_mapping_values, symbol_info, symbol_info_tick, symbols, @@ -2436,3 +2437,263 @@ class TestThrottledHistoryUpdater: updater.update(MagicMock(), ["EURUSD"]) assert updater.last_update_monotonic is None + + +class TestBuildConfigStringLogin: + """Tests for build_config() string login coercion (issue #61).""" + + @pytest.mark.parametrize( + ("login", "expected"), + [ + (None, None), + (12345, 12345), + ("12345", 12345), + (" 12345 ", 12345), + ("", None), + (" ", None), + ], + ) + def test_coerces_login_from_string( + self, + login: int | str | None, + expected: int | None, + ) -> None: + """Test build_config coerces string login to int or None.""" + config = build_config(login=login) + assert config.login == expected + + def test_rejects_non_numeric_string_login(self) -> None: + """Test build_config raises ValueError for non-numeric string login.""" + with pytest.raises(ValueError, match="invalid literal"): + build_config(login="abc") + + def test_expands_dollar_brace_login_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test build_config expands ${MT5_LOGIN} and coerces with opt-in.""" + monkeypatch.setenv("MT5_LOGIN", "12345") + config = build_config(login="${MT5_LOGIN}", allow_whole_dollar_env=True) + assert config.login == 12345 + + def test_expands_whole_dollar_login_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test build_config expands $MT5_LOGIN and coerces with opt-in.""" + monkeypatch.setenv("MT5_LOGIN", "99999") + config = build_config(login="$MT5_LOGIN", allow_whole_dollar_env=True) + assert config.login == 99999 + + def test_missing_env_variable_raises( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test build_config raises ValueError when referenced env var is not set.""" + monkeypatch.delenv("MT5_LOGIN", raising=False) + with pytest.raises(ValueError, match="'MT5_LOGIN' is not set"): + build_config(login="${MT5_LOGIN}", allow_whole_dollar_env=True) + + def test_env_expands_to_blank_becomes_none( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test build_config coerces blank env-expanded login to None.""" + monkeypatch.setenv("MT5_LOGIN", "") + config = build_config(login="${MT5_LOGIN}", allow_whole_dollar_env=True) + assert config.login is None + + def test_dollar_brace_login_not_expanded_without_opt_in(self) -> None: + """Test ${MT5_LOGIN} is not expanded when allow_whole_dollar_env=False.""" + with pytest.raises(ValueError, match="invalid literal"): + build_config(login="${MT5_LOGIN}") + + def test_integer_login_preserved_backward_compat(self) -> None: + """Test existing int login callers remain backward-compatible.""" + config = build_config(login=54321) + assert config.login == 54321 + + def test_none_login_preserved_backward_compat(self) -> None: + """Test existing None login callers remain backward-compatible.""" + config = build_config(login=None) + assert config.login is None + + +class TestSubstituteMappingValues: + """Tests for substitute_mapping_values() (issue #62).""" + + def test_substitutes_selected_keys_in_flat_dict( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test selected keys are substituted in a flat mapping.""" + monkeypatch.setenv("MT5_LOGIN", "12345") + data: dict[str, object] = { + "mt5_login": "${MT5_LOGIN}", + "strategy_name": "${MT5_LOGIN}", + } + result = substitute_mapping_values(data, keys={"mt5_login"}) + assert result == {"mt5_login": "12345", "strategy_name": "${MT5_LOGIN}"} + + def test_preserves_non_selected_literal_dollar_signs( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test literal dollar signs in non-selected fields are preserved exactly.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + data: dict[str, object] = { + "mt5_password": "${MT5_PASSWORD}", + "notes": "$NOT_EXPANDED", + } + result = substitute_mapping_values(data, keys={"mt5_password"}) + assert result == {"mt5_password": "secret", "notes": "$NOT_EXPANDED"} + + def test_nested_dict_traversal_substitutes_selected_keys( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test selected keys inside nested dicts are substituted.""" + monkeypatch.setenv("MT5_SERVER", "Broker-Demo") + data: dict[str, object] = { + "outer": { + "mt5_server": "${MT5_SERVER}", + "other": "${MT5_SERVER}", + } + } + result = substitute_mapping_values(data, keys={"mt5_server"}) + assert result == { + "outer": {"mt5_server": "Broker-Demo", "other": "${MT5_SERVER}"} + } + + def test_nested_list_traversal_substitutes_selected_keys( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test selected keys inside list elements are substituted.""" + monkeypatch.setenv("MT5_LOGIN", "42") + data: dict[str, object] = { + "accounts": [ + {"mt5_login": "${MT5_LOGIN}", "name": "${MT5_LOGIN}"}, + {"mt5_login": "${MT5_LOGIN}", "name": "fixed"}, + ] + } + result = substitute_mapping_values(data, keys={"mt5_login"}) + assert result == { + "accounts": [ + {"mt5_login": "42", "name": "${MT5_LOGIN}"}, + {"mt5_login": "42", "name": "fixed"}, + ] + } + + def test_whole_dollar_expanded_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test $ENV_NAME is expanded when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + data: dict[str, object] = {"mt5_password": "$MT5_PASSWORD"} + result = substitute_mapping_values( + data, + keys={"mt5_password"}, + allow_whole_dollar_env=True, + ) + assert result == {"mt5_password": "secret"} + + def test_whole_dollar_not_expanded_by_default( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test $ENV_NAME in a selected key is preserved when opt-in is False.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + data: dict[str, object] = {"mt5_password": "$MT5_PASSWORD"} + result = substitute_mapping_values(data, keys={"mt5_password"}) + assert result == {"mt5_password": "$MT5_PASSWORD"} + + def test_blank_string_becomes_none_for_blank_keys(self) -> None: + """Test blank strings are normalised to None for blank_string_keys_as_none.""" + data: dict[str, object] = { + "mt5_login": "", + "mt5_password": " ", + "other": "", + } + result = substitute_mapping_values( + data, + keys=set(), + blank_string_keys_as_none={"mt5_login", "mt5_password"}, + ) + assert result == {"mt5_login": None, "mt5_password": None, "other": ""} + + def test_env_expanded_blank_becomes_none( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test env-expanded blank string is normalised to None.""" + monkeypatch.setenv("MT5_LOGIN", "") + data: dict[str, object] = {"mt5_login": "${MT5_LOGIN}"} + result = substitute_mapping_values( + data, + keys={"mt5_login"}, + blank_string_keys_as_none={"mt5_login"}, + ) + assert result == {"mt5_login": None} + + def test_missing_env_variable_raises_for_selected_key( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test missing env var for a selected key raises ValueError.""" + monkeypatch.delenv("MT5_MISSING", raising=False) + data: dict[str, object] = {"mt5_login": "${MT5_MISSING}"} + with pytest.raises(ValueError, match="'MT5_MISSING' is not set"): + substitute_mapping_values(data, keys={"mt5_login"}) + + def test_non_string_values_preserved(self) -> None: + """Test non-string values under selected or non-selected keys are preserved.""" + data: dict[str, object] = { + "mt5_login": 12345, + "timeout": 5000, + "enabled": True, + "ratio": 1.5, + "nothing": None, + } + result = substitute_mapping_values( + data, keys={"mt5_login", "timeout", "enabled", "ratio", "nothing"} + ) + assert result == data + + def test_caller_supplied_key_set_substitutes_correctly( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test helper works with any caller-supplied key set.""" + monkeypatch.setenv("APP_LOGIN", "77777") + monkeypatch.setenv("APP_PASSWORD", "p4ss") + data: dict[str, object] = { + "app_login": "${APP_LOGIN}", + "app_password": "${APP_PASSWORD}", + "unrelated": "${APP_LOGIN}", + } + credential_keys = {"app_login", "app_password"} + result = substitute_mapping_values(data, keys=credential_keys) + assert result == { + "app_login": "77777", + "app_password": "p4ss", + "unrelated": "${APP_LOGIN}", + } + + def test_scalar_data_returned_unchanged(self) -> None: + """Test a scalar (non-dict, non-list) value is returned as-is.""" + assert substitute_mapping_values("hello", keys={"x"}) == "hello" + assert substitute_mapping_values(42, keys={"x"}) == 42 + assert substitute_mapping_values(None, keys={"x"}) is None + + def test_tuple_container_not_traversed( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test tuple containers are returned as-is without traversal.""" + monkeypatch.setenv("MT5_LOGIN", "42") + data: dict[str, object] = {"accounts": ({"mt5_login": "${MT5_LOGIN}"},)} + result = substitute_mapping_values(data, keys={"mt5_login"}) + # tuple is returned as-is; inner dict is NOT visited + assert result == {"accounts": ({"mt5_login": "${MT5_LOGIN}"},)} diff --git a/uv.lock b/uv.lock index 7d5407b..e1d1247 100644 --- a/uv.lock +++ b/uv.lock @@ -487,7 +487,7 @@ wheels = [ [[package]] name = "mt5cli" -version = "0.9.4" +version = "0.9.5" source = { editable = "." } dependencies = [ { name = "click" },