MPT-17614 validate, clean base url

[jentyk]

Closes MPT-17614

• Add validate_base_url() in mpt_api_client/http/client_utils.py to centralize base URL validation and normalization.
• validate_base_url:
  • Requires a non-empty string, defaults scheme to https, requires a hostname.
  • Strips userinfo, handles ports (including IPv6 bracket-wrapping), and removes specific path prefixes ("/", "/public/", "/public/v1/").
  • Raises ValueError with a clear message for invalid inputs.
• Replace manual base URL retrieval/validation in HTTPClient and AsyncHTTPClient with validate_base_url(base_url or os.getenv("MPT_URL")) (removes prior in-place missing-URL ValueError).
• Add unit tests tests/unit/http/test_client_utils.py covering IPv6, userinfo stripping, path normalization, scheme defaults, and invalid inputs.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Centralizes base URL validation into a new utility validate_base_url() and updates HTTP client modules to use it; adds comprehensive unit tests for the validator covering normalization and invalid inputs.

Changes

Cohort / File(s)	Summary
Client modules mpt_api_client/http/async_client.py, mpt_api_client/http/client.py	Import and call validate_base_url(base_url or os.getenv("MPT_URL")), removing inlined retrieval and explicit None-check/ValueError logic.
Validation utility mpt_api_client/http/client_utils.py	New module adding INVALID_ENV_URL_MESSAGE, PATHS_TO_REMOVE_RE, helper functions for host/port/path formatting, and public validate_base_url(base_url: str) that validates and normalizes/sanitizes base URLs (scheme, hostname, IPv6, ports, credential stripping, and path cleanup).
Tests tests/unit/http/test_client_utils.py	Adds parameterized tests for URL normalization (default scheme, credential removal, path stripping, IPv6 and port handling) and tests asserting invalid/empty inputs raise ValueError.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (2 files): ⚔️ mpt_api_client/http/async_client.py (content) ⚔️ mpt_api_client/http/client.py (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Jira Issue Key In Title	✅ Passed	The PR title contains exactly one Jira issue key (MPT-17614) in the correct format.
Test Coverage Required	✅ Passed	PR modifies 3 code files and includes corresponding test coverage with 42 lines of parameterized tests for validate_base_url() function.
Single Commit Required	✅ Passed	The PR contains exactly one commit (0341992: fix: validate, clean base url), which meets the requirement for a single commit.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@mpt_api_client/http/client_utils.py`:
- Around line 11-16: The inline multi-line error message used in the ValueError
when validating base_url should be moved to a module-level constant to satisfy
TRY003; create a descriptive constant (e.g. BASE_URL_ERROR_MSG) at the top of
the module and replace the inline string in the raise ValueError(...) (the check
that inspects base_url before constructing MPTClient) with that constant so the
ValueError is raised with the constant variable instead of a raw multi-line
literal.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@mpt_api_client/http/client_utils.py`:
- Around line 17-24: The code accesses split_result.port which can raise
ValueError for malformed ports and bypass the standard INVALID_ENV_URL_MESSAGE;
update the logic around port handling in the function that builds the sanitized
base URL (the block using split_result, host, port, PATH_TO_REMOVE_RE, and
urlunparse) to guard access to split_result.port with a try/except ValueError,
normalize the exception by raising or returning the same INVALID_ENV_URL_MESSAGE
path/validation flow, and then continue to construct host/port/path and call
urlunparse as before so malformed ports produce the consistent validation error.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@mpt_api_client/http/client_utils.py`:
- Around line 13-29: The validate_base_url function is over the complexity
limit; extract the URL reconstruction logic into a small helper (e.g.,
build_normalized_url or format_host_port_path) that takes the urlsplit result
and PATH_TO_REMOVE_RE and returns the normalized URL string; leave
validate_base_url responsible only for input validation, calling urlsplit,
delegating host/port/path formatting to the helper, keeping the existing
try/except for split_result.port and re-raising ValueError as before, and ensure
you still use urlsplit, urlunparse, split_result.hostname/port/scheme and
PATH_TO_REMOVE_RE so behavior is unchanged.

🧹 Nitpick comments (1)

tests/unit/http/test_client_utils.py (1)

30-42: Good error case coverage; consider adding malformed port tests.

The test cases cover empty and malformed URL structures well. Given that validate_base_url includes a try/except block specifically for port parsing errors (lines 22-25 in client_utils.py), consider adding test cases for malformed ports to verify the error handling:

"http://example.com:abc",     # Non-numeric port
"http://example.com:99999",   # Port out of range

[d3rky]

Could you pls try to avoid using Any here, I'm pretty sure there should be a proper type of url here

[jentyk]

done

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@mpt_api_client/http/client_utils.py`:
- Around line 35-44: Update the validate_base_url function signature to accept
an optional URL by changing its parameter type to Optional[str] (import Optional
from typing) so callers like base_url or os.getenv("MPT_URL") have a compatible
type; keep the return type as str and leave the internal runtime checks (raising
ValueError for None/invalid) unchanged; update any related type hints/usages of
validate_base_url if necessary to reflect Optional[str] input.
- Around line 1-32: The type annotations for _format_port and
_build_sanitized_base_url are incorrect: import SplitResult from urllib.parse
and change their parameter types from str to SplitResult; in
_build_sanitized_base_url, guard against split_result.hostname being None (e.g.,
raise ValueError(INVALID_ENV_URL_MESSAGE) or handle it) before calling
_format_host, and keep accessing split_result.port, .path and .scheme as before
so mypy/type checkers accept the code.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud