Skip to content

Commit cf49d2a

Browse files
authored
[MPT-18559] Adopt api client to audit and remove legacy mpt client (#168)
This pull request removes the legacy `MPTClient` implementation and its usage throughout the codebase, fully migrating to the new `mpt_api_client` package for Marketplace API interactions. All relevant application logic, dependency injection, and tests have been updated to use the new client, and the audit plugin API has been refactored to leverage the new query interface, resulting in cleaner and more maintainable code. Migration to new Marketplace API client: * Completely removed the legacy `cli.core.mpt.client.py` file and its `MPTClient` and `client_from_account` implementations. All references to these have been replaced with the new `mpt_api_client` package and its `MPTClient` class. [[1]](diffhunk://#diff-c1556732cf26919aa027ba9ed3ba39c2b8b3ba410235d15aed36470c9692b281L1-L85) [[2]](diffhunk://#diff-31fd81db50cff29b241df3338586dd2dd49ff3f932daf686ec2101da4b913c19L2) [[3]](diffhunk://#diff-31fd81db50cff29b241df3338586dd2dd49ff3f932daf686ec2101da4b913c19L19) [[4]](diffhunk://#diff-8b37c37e1e6e8f71c9be14b72d40e0dfac87932b49653cebde9f1e1389b1b66bL5) [[5]](diffhunk://#diff-1a492288536e42ccf8b3e90c7aa18c7968c654a4d31110067a13cef6fb1759ffL1-L80) * Updated dependency injection in `AccountContainer` to provide only the new API client via `create_api_mpt_client_from_account`, removing the legacy client provider. * Refactored all application code (`audit_plugin/app.py`) and tests to use the new client creation function and updated mocks accordingly. [[1]](diffhunk://#diff-0a11b924632def89cff0ee0e05f39cdcf601c928bb012ff09fc8b9e403b114bfL6-R6) [[2]](diffhunk://#diff-0a11b924632def89cff0ee0e05f39cdcf601c928bb012ff09fc8b9e403b114bfL95-R95) [[3]](diffhunk://#diff-0a11b924632def89cff0ee0e05f39cdcf601c928bb012ff09fc8b9e403b114bfL142-R142) [[4]](diffhunk://#diff-8b37c37e1e6e8f71c9be14b72d40e0dfac87932b49653cebde9f1e1389b1b66bL16-L20) [[5]](diffhunk://#diff-8b37c37e1e6e8f71c9be14b72d40e0dfac87932b49653cebde9f1e1389b1b66bL99) [[6]](diffhunk://#diff-66fbbcba009b62a06d587e17f8f0470f686d5170cf10002d0c2e2aaea32eb708L24-L30) [[7]](diffhunk://#diff-66fbbcba009b62a06d587e17f8f0470f686d5170cf10002d0c2e2aaea32eb708L50-L56) [[8]](diffhunk://#diff-1b352cf74ff12715226e02ca5c895db1b150661ba1373af300c5cd756182c555L38-R43) [[9]](diffhunk://#diff-1b352cf74ff12715226e02ca5c895db1b150661ba1373af300c5cd756182c555L52-R63) [[10]](diffhunk://#diff-1b352cf74ff12715226e02ca5c895db1b150661ba1373af300c5cd756182c555L79-R82) Audit plugin API improvements: * Refactored `get_audit_trail` and `get_audit_records_by_object` to use the new query interface (`options`, `filter`, `select`, `fetch_page`) instead of manually constructing query strings and endpoints, resulting in more robust and readable code. * Updated exception handling and error messages to be clearer and more consistent in the audit plugin API. Test suite updates: * Removed all tests for the legacy client and replaced or updated tests to mock the new client and its query interface, including changes to fixtures and patching. [[1]](diffhunk://#diff-1a492288536e42ccf8b3e90c7aa18c7968c654a4d31110067a13cef6fb1759ffL1-L80) [[2]](diffhunk://#diff-51860b275c432c506213dfa8fee143965dfc1aeecfe4d38439e06f234b7d6e33L2-R96) [[3]](diffhunk://#diff-8b37c37e1e6e8f71c9be14b72d40e0dfac87932b49653cebde9f1e1389b1b66bL16-L20) [[4]](diffhunk://#diff-8b37c37e1e6e8f71c9be14b72d40e0dfac87932b49653cebde9f1e1389b1b66bL99) [[5]](diffhunk://#diff-1b352cf74ff12715226e02ca5c895db1b150661ba1373af300c5cd756182c555L38-R43) [[6]](diffhunk://#diff-1b352cf74ff12715226e02ca5c895db1b150661ba1373af300c5cd756182c555L52-R63) [[7]](diffhunk://#diff-1b352cf74ff12715226e02ca5c895db1b150661ba1373af300c5cd756182c555L79-R82) [[1]](diffhunk://#diff-c1556732cf26919aa027ba9ed3ba39c2b8b3ba410235d15aed36470c9692b281L1-L85) [[2]](diffhunk://#diff-31fd81db50cff29b241df3338586dd2dd49ff3f932daf686ec2101da4b913c19L2) [[3]](diffhunk://#diff-31fd81db50cff29b241df3338586dd2dd49ff3f932daf686ec2101da4b913c19L19) [[4]](diffhunk://#diff-440d64ab776a9667c05fb518f6d3ab5b653172b8515f390e4fe4bbb9a125dfa7R5-R50) [[5]](diffhunk://#diff-1a492288536e42ccf8b3e90c7aa18c7968c654a4d31110067a13cef6fb1759ffL1-L80) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> Closes [MPT-18559](https://softwareone.atlassian.net/browse/MPT-18559) - Remove legacy MPTClient implementation and factory (cli/core/mpt/client.py) and delete associated unit tests. - Replace legacy client usage with mpt_api_client.MPTClient and its fluent query/RQL API (options(), filter(), select(), fetch_page(), RQLQuery). - Update dependency injection: remove legacy mpt_client provider from AccountContainer and use create_api_mpt_client_from_account (cli/core/mpt/mpt_client) to provide the new client. - Refactor audit plugin API (cli/plugins/audit_plugin/api.py) to use the new client/query interface, select explicit fields, and standardize error handling (Exit on missing records or API errors). - Update audit plugin app code (cli/plugins/audit_plugin/app.py) to instantiate the new client via create_api_mpt_client_from_account. - Adjust tests and fixtures: remove legacy client fixtures and tests, mock the new MPTClient fluent API and RQLQuery, migrate patching/imports from cli.core.mpt.client to cli.core.mpt.mpt_client and mpt_api_client, and update test flows accordingly. <!-- end of auto-generated comment: release notes by coderabbit.ai --> [MPT-18559]: https://softwareone.atlassian.net/browse/MPT-18559?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
2 parents 5ef5fc8 + 17bc5a3 commit cf49d2a

File tree

9 files changed

+225
-429
lines changed

9 files changed

+225
-429
lines changed

cli/core/accounts/containers.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
from cli.core.accounts.app import get_active_account
2-
from cli.core.mpt.client import client_from_account
32
from cli.core.mpt.mpt_client import create_api_mpt_client_from_account
43
from dependency_injector import containers, providers
54

@@ -16,5 +15,4 @@ class AccountContainer(containers.DeclarativeContainer):
1615
"""
1716

1817
account = providers.Singleton(get_active_account)
19-
mpt_client = providers.Singleton(client_from_account, account)
2018
api_mpt_client = providers.Singleton(create_api_mpt_client_from_account, account)

cli/core/mpt/client.py

Lines changed: 0 additions & 85 deletions
This file was deleted.

cli/plugins/audit_plugin/api.py

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,43 +2,52 @@
22

33
import typer
44
from cli.core.console import console
5+
from mpt_api_client import MPTClient, RQLQuery
56

67

7-
def get_audit_trail(client: Any, record_id: str) -> dict[str, Any]:
8+
def get_audit_trail(client: MPTClient, record_id: str) -> dict[str, Any]:
89
"""Retrieve audit trail for a specific record."""
9-
endpoint = f"/audit/records/{record_id}"
10-
query_string = "render()&select=object,actor,details,documents,request.api.geolocation"
11-
1210
try:
13-
response = client.get(f"{endpoint}?{query_string}")
14-
return response.json()
11+
# TODO: Using select instead of get because render() query string isn't working with get().
12+
select_fields = ["object", "actor", "details", "documents", "request.api.geolocation"]
13+
audit_collection = client.audit.records.options(render=True)
14+
audit_collection_filtered = audit_collection.filter(RQLQuery(id=record_id))
15+
records = audit_collection_filtered.select(*select_fields).fetch_page(limit=1)
1516
except Exception as error:
1617
console.print(
1718
f"[red]Failed to retrieve audit trail for record {record_id}: {error!s}[/red]"
1819
)
20+
raise typer.Exit(1) from error
21+
22+
if not records:
23+
console.print(f"[red]No audit record found with ID {record_id}[/red]")
1924
raise typer.Exit(1)
2025

26+
return records[0].to_dict()
27+
2128

2229
def get_audit_records_by_object(
2330
client: Any, object_id: str, limit: int = 10
2431
) -> list[dict[str, Any]]:
2532
"""Retrieve all audit records for a specific object."""
26-
endpoint = "/audit/records"
27-
query_string = (
28-
"render()&select=object,actor,details,documents,request.api.geolocation"
29-
f"&eq(object.id,'{object_id}')&order=-timestamp&limit={limit}"
30-
)
31-
3233
try:
33-
response = client.get(f"{endpoint}?{query_string}")
34-
records = response.json().get("data", [])
35-
if not records:
36-
console.print(f"[red]No audit records found for object {object_id}[/red]")
37-
raise typer.Exit(1) # noqa: TRY301
34+
select_fields = ["object", "actor", "details", "documents", "request.api.geolocation"]
35+
object_id_filter = RQLQuery(object__id=object_id)
36+
order_by = "-timestamp"
37+
audit_records_collection = client.audit.records.options(render=True)
38+
audit_records_filtered = audit_records_collection.filter(object_id_filter)
39+
raw_records = (
40+
audit_records_filtered.order_by(order_by).select(*select_fields).fetch_page(limit=limit)
41+
)
42+
records = [raw_record.to_dict() for raw_record in raw_records]
3843
except Exception as error:
3944
console.print(
4045
f"[red]Failed to retrieve audit records for object {object_id}: {error!s}[/red]"
4146
)
47+
raise typer.Exit(1) from error
48+
49+
if not records:
50+
console.print(f"[red]No audit records found for object {object_id}[/red]")
4251
raise typer.Exit(1)
4352

4453
return records

cli/plugins/audit_plugin/app.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import typer
44
from cli.core.accounts.app import get_active_account
55
from cli.core.console import console
6-
from cli.core.mpt.client import client_from_account
6+
from cli.core.mpt.mpt_client import create_api_mpt_client_from_account
77
from cli.plugins.audit_plugin.api import get_audit_records_by_object, get_audit_trail
88
from cli.plugins.audit_plugin.audit_records import (
99
display_audit_records,
@@ -92,7 +92,7 @@ def diff_by_object_id(
9292
9393
"""
9494
account = get_active_account()
95-
client = client_from_account(account)
95+
client = create_api_mpt_client_from_account(account)
9696

9797
records = get_audit_records_by_object(client, object_id, limit)
9898
if len(records) < 2:
@@ -139,7 +139,7 @@ def diff_by_records_id(
139139
140140
"""
141141
account = get_active_account()
142-
client = client_from_account(account)
142+
client = create_api_mpt_client_from_account(account)
143143

144144
source_trail = get_audit_trail(client, source)
145145
target_trail = get_audit_trail(client, target)

tests/cli/core/conftest.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import responses
33
from cli.core.accounts.containers import AccountContainer
44
from cli.core.accounts.models import Account as CLIAccount
5-
from cli.core.mpt.client import MPTClient as LegacyMPTClient
65
from mpt_api_client import MPTClient
76

87

@@ -13,11 +12,6 @@ def requests_mocker():
1312
yield rsps
1413

1514

16-
@pytest.fixture
17-
def mpt_client():
18-
return LegacyMPTClient("https://example.com", "token")
19-
20-
2115
@pytest.fixture
2216
def mock_mpt_api_client(mocker):
2317
return mocker.MagicMock(spec=MPTClient)
@@ -96,7 +90,6 @@ def active_vendor_account():
9690
def account_container_mock(mocker, active_operations_account):
9791
container = AccountContainer()
9892
container.account.override(mocker.MagicMock(return_value=active_operations_account))
99-
container.mpt_client.override(mocker.MagicMock(spec=LegacyMPTClient))
10093
container.api_mpt_client.override(mocker.MagicMock(spec=MPTClient))
10194
mock = mocker.patch("cli.core.products.app.AccountContainer", autospec=True)
10295
mock.return_value = container

tests/cli/core/mpt/test_client.py

Lines changed: 0 additions & 80 deletions
This file was deleted.

tests/cli/core/test_debug_logging.py

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,8 @@ def mock_basic_config(**kwargs):
2121
root_logger.debug("Test debug message")
2222

2323

24-
def test_log_file_writes_to_file(tmp_path, mocker, active_vendor_account):
24+
def test_log_file_writes_to_file(tmp_path, mocker):
2525
mocker.patch("cli.core.accounts.app.list_accounts", return_value=[], autospec=True)
26-
mocker.patch(
27-
"cli.core.mpt.client.client_from_account",
28-
return_value=active_vendor_account,
29-
autospec=True,
30-
)
3126
log_file = tmp_path / "test.log"
3227
mocker.patch("logging.basicConfig", side_effect=mock_basic_config, autospec=True)
3328

@@ -47,13 +42,8 @@ def test_verbose_and_log_file_are_exclusive():
4742
assert "Cannot use both --verbose and --log-file together" in result.stdout
4843

4944

50-
def test_log_file_creates_parent_directories(tmp_path, mocker, active_vendor_account):
45+
def test_log_file_creates_parent_directories(tmp_path, mocker):
5146
mocker.patch("cli.core.accounts.app.list_accounts", return_value=[], autospec=True)
52-
mocker.patch(
53-
"cli.core.mpt.client.client_from_account",
54-
return_value=active_vendor_account,
55-
autospec=True,
56-
)
5747
mocker.patch("logging.basicConfig", side_effect=mock_basic_config, autospec=True)
5848
nested_log_file = tmp_path / "nested" / "dir" / "test.log"
5949

0 commit comments

Comments
 (0)