From 54bfedc630374fd5483b8fcaf81058310826e363 Mon Sep 17 00:00:00 2001 From: Arthur Tonial Date: Thu, 30 Jul 2026 22:48:24 -0300 Subject: [PATCH 1/2] Revert "feat(agent-memory): multitenancy via reusable XSUAA service (#222)" This reverts commit d418d8a2e3d47f50cc931dcac18b64c2bdb00e87. --- docs/INTEGRATION_TESTS.md | 3 - pyproject.toml | 2 +- src/sap_cloud_sdk/agent_memory/__init__.py | 12 +- src/sap_cloud_sdk/agent_memory/client.py | 59 ++---- src/sap_cloud_sdk/agent_memory/config.py | 27 ++- src/sap_cloud_sdk/agent_memory/user-guide.md | 3 +- tests/agent_memory/integration/conftest.py | 39 +++- tests/agent_memory/unit/test_client.py | 186 ++++-------------- tests/agent_memory/unit/test_config.py | 77 +++++++- .../data_anonymization/test_http_transport.py | 4 +- uv.lock | 2 +- 11 files changed, 204 insertions(+), 210 deletions(-) diff --git a/docs/INTEGRATION_TESTS.md b/docs/INTEGRATION_TESTS.md index 467f0666..133247f4 100644 --- a/docs/INTEGRATION_TESTS.md +++ b/docs/INTEGRATION_TESTS.md @@ -75,9 +75,6 @@ For Agent Memory integration tests, configure the following variables in `.env_i # Agent Memory Configuration CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL=https://your-agent-memory-api-url CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA='{"url":"https://your-auth-url","clientid":"your-client-id","clientsecret":"your-client-secret"}' - -## For the multi-tenant scenarios, set the subscriber tenant subdomain. When not set, those scenarios are automatically skipped -CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT=your-subscriber-tenant-subdomain-here ``` ### AuditLog Integration Tests diff --git a/pyproject.toml b/pyproject.toml index 19a01bee..e1e9a5a2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sap-cloud-sdk" -version = "0.40.0" +version = "0.39.0" description = "SAP Cloud SDK for Python" readme = "README.md" license = "Apache-2.0" diff --git a/src/sap_cloud_sdk/agent_memory/__init__.py b/src/sap_cloud_sdk/agent_memory/__init__.py index a67b6433..6177df68 100644 --- a/src/sap_cloud_sdk/agent_memory/__init__.py +++ b/src/sap_cloud_sdk/agent_memory/__init__.py @@ -21,6 +21,7 @@ from sap_cloud_sdk.agent_memory.client import AgentMemoryClient from sap_cloud_sdk.agent_memory.config import ( AgentMemoryConfig, + _load_config_for_instance, _load_config_from_env, ) from sap_cloud_sdk.agent_memory.exceptions import ( @@ -51,11 +52,12 @@ def create_client( The binding loaded depends on ``access_strategy`` and ``tenant``: - - ``SUBSCRIBER`` with ``tenant="acme-corp"`` — loads credentials from + - ``SUBSCRIBER`` with ``tenant="acme-corp"`` — loads the subscriber + binding from ``/etc/secrets/appfnd/hana-agent-memory/acme-corp/`` (or + ``CLOUD_SDK_CFG_HANA_AGENT_MEMORY_ACME_CORP_*`` env vars). + - ``PROVIDER`` — loads the provider binding from ``/etc/secrets/appfnd/hana-agent-memory/default/`` (or - ``CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_*`` env vars) and derives the - subscriber token URL using the ``identityzone`` field. - - ``PROVIDER`` — same binding, uses provider token directly. + ``CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_*`` env vars). - Explicit ``config`` — uses the provided configuration directly. Args: @@ -77,6 +79,8 @@ def create_client( try: if config is not None: resolved_config = config + elif access_strategy is AccessStrategy.SUBSCRIBER and tenant: + resolved_config = _load_config_for_instance(tenant) else: resolved_config = _load_config_from_env() diff --git a/src/sap_cloud_sdk/agent_memory/client.py b/src/sap_cloud_sdk/agent_memory/client.py index c0d4e6e3..9e28ac54 100644 --- a/src/sap_cloud_sdk/agent_memory/client.py +++ b/src/sap_cloud_sdk/agent_memory/client.py @@ -77,14 +77,13 @@ class AgentMemoryClient: Do not instantiate directly — use :func:`sap_cloud_sdk.agent_memory.create_client`. Args: - transport: HTTP transport loaded from the default service binding - (resolved once at construction time by + transport: HTTP transport loaded from the binding for the configured + access strategy and tenant (resolved once at construction time by :func:`sap_cloud_sdk.agent_memory.create_client`). access_strategy: Tenant access strategy for all operations. Defaults to ``SUBSCRIBER``. tenant: Subscriber tenant subdomain. Required when - ``access_strategy=SUBSCRIBER``. The subscriber token URL is - derived from the provider binding's ``identityzone`` field. + ``access_strategy=SUBSCRIBER``. """ def __init__( @@ -104,7 +103,6 @@ def __init__( "Only use this strategy for provider-owned operations." ) self._transport = transport - self._tenant = tenant if access_strategy is AccessStrategy.SUBSCRIBER else None def close(self) -> None: """Close the underlying HTTP session and release resources.""" @@ -150,9 +148,7 @@ def add_memory( } if metadata is not None: payload["metadata"] = metadata - data = self._transport.post( - MEMORIES, json=payload, tenant_subdomain=self._tenant - ) + data = self._transport.post(MEMORIES, json=payload) return Memory.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_GET_MEMORY) @@ -171,9 +167,7 @@ def get_memory(self, memory_id: str) -> Memory: AgentMemoryHttpError: If the request fails. """ _require_non_empty(memory_id=memory_id) - data = self._transport.get( - f"{MEMORIES}({memory_id})", tenant_subdomain=self._tenant - ) + data = self._transport.get(f"{MEMORIES}({memory_id})") return Memory.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_UPDATE_MEMORY) @@ -207,9 +201,7 @@ def update_memory( payload["content"] = content if metadata is not None: payload["metadata"] = metadata - self._transport.patch( - f"{MEMORIES}({memory_id})", json=payload, tenant_subdomain=self._tenant - ) + self._transport.patch(f"{MEMORIES}({memory_id})", json=payload) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_DELETE_MEMORY) def delete_memory(self, memory_id: str) -> None: @@ -224,9 +216,7 @@ def delete_memory(self, memory_id: str) -> None: AgentMemoryHttpError: If the request fails. """ _require_non_empty(memory_id=memory_id) - self._transport.delete( - f"{MEMORIES}({memory_id})", tenant_subdomain=self._tenant - ) + self._transport.delete(f"{MEMORIES}({memory_id})") @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_LIST_MEMORIES) def list_memories( @@ -274,9 +264,7 @@ def list_memories( top=limit, skip=offset if offset else None, ) - response = self._transport.get( - MEMORIES, params=params, tenant_subdomain=self._tenant - ) + response = self._transport.get(MEMORIES, params=params) items, _ = extract_value_and_count(response) return [Memory.from_dict(item) for item in items] @@ -301,9 +289,7 @@ def count_memories( top=0, count=True, ) - response = self._transport.get( - MEMORIES, params=params, tenant_subdomain=self._tenant - ) + response = self._transport.get(MEMORIES, params=params) _, total = extract_value_and_count(response) return total or 0 @@ -350,9 +336,7 @@ def search_memories( "threshold": threshold, "top": limit, } - response = self._transport.post( - MEMORY_SEARCH, json=payload, tenant_subdomain=self._tenant - ) + response = self._transport.post(MEMORY_SEARCH, json=payload) items = response.get("value", []) return [SearchResult.from_dict(item) for item in items] @@ -404,9 +388,7 @@ def add_message( } if metadata is not None: payload["metadata"] = metadata - data = self._transport.post( - MESSAGES, json=payload, tenant_subdomain=self._tenant - ) + data = self._transport.post(MESSAGES, json=payload) return Message.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_GET_MESSAGE) @@ -425,9 +407,7 @@ def get_message(self, message_id: str) -> Message: AgentMemoryHttpError: If the request fails. """ _require_non_empty(message_id=message_id) - data = self._transport.get( - f"{MESSAGES}({message_id})", tenant_subdomain=self._tenant - ) + data = self._transport.get(f"{MESSAGES}({message_id})") return Message.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_DELETE_MESSAGE) @@ -443,9 +423,7 @@ def delete_message(self, message_id: str) -> None: AgentMemoryHttpError: If the request fails. """ _require_non_empty(message_id=message_id) - self._transport.delete( - f"{MESSAGES}({message_id})", tenant_subdomain=self._tenant - ) + self._transport.delete(f"{MESSAGES}({message_id})") @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_LIST_MESSAGES) def list_messages( @@ -499,9 +477,7 @@ def list_messages( top=limit, skip=offset if offset else None, ) - response = self._transport.get( - MESSAGES, params=params, tenant_subdomain=self._tenant - ) + response = self._transport.get(MESSAGES, params=params) items, _ = extract_value_and_count(response) return [Message.from_dict(item) for item in items] @@ -517,10 +493,9 @@ def get_retention_config(self) -> RetentionConfig: The current :class:`RetentionConfig`. Raises: - AgentMemoryValidationError: If tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ - data = self._transport.get(RETENTION_CONFIG, tenant_subdomain=self._tenant) + data = self._transport.get(RETENTION_CONFIG) return RetentionConfig.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_UPDATE_RETENTION_CONFIG) @@ -566,6 +541,4 @@ def update_retention_config( payload["memoryDays"] = memory_days if usage_log_days is not None: payload["usageLogDays"] = usage_log_days - self._transport.patch( - RETENTION_CONFIG, json=payload, tenant_subdomain=self._tenant - ) + self._transport.patch(RETENTION_CONFIG, json=payload) diff --git a/src/sap_cloud_sdk/agent_memory/config.py b/src/sap_cloud_sdk/agent_memory/config.py index a5621b44..37569002 100644 --- a/src/sap_cloud_sdk/agent_memory/config.py +++ b/src/sap_cloud_sdk/agent_memory/config.py @@ -134,6 +134,29 @@ def _load_config_from_env() -> AgentMemoryConfig: 1. Mount at ``/etc/secrets/appfnd/hana-agent-memory/default/`` 2. Environment variables ``CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_*`` + Returns: + A validated ``AgentMemoryConfig``. + + Raises: + AgentMemoryConfigError: If configuration cannot be loaded or is incomplete. + """ + return _load_config_for_instance("default") + + +def _load_config_for_instance(instance: str) -> AgentMemoryConfig: + """Load Agent Memory configuration for a named binding instance. + + Uses the secret resolver with fallback order: + 1. Mount at ``/etc/secrets/appfnd/hana-agent-memory/{instance}/`` + 2. Environment variables ``CLOUD_SDK_CFG_HANA_AGENT_MEMORY_{INSTANCE}_*`` + + This is used to load tenant-specific bindings when the runtime provisions + a dedicated service instance per subscriber tenant. + + Args: + instance: The binding instance name — ``"default"`` for the provider, + or a tenant subdomain for a subscriber (e.g. ``"acme-corp"``). + Returns: A validated ``AgentMemoryConfig``. @@ -150,7 +173,7 @@ def _load_config_from_env() -> AgentMemoryConfig: base_volume_mount="/etc/secrets/appfnd", base_var_name="CLOUD_SDK_CFG", module="hana-agent-memory", - instance="default", + instance=instance, target=binding, ) binding.validate() @@ -159,5 +182,5 @@ def _load_config_from_env() -> AgentMemoryConfig: raise except Exception as exc: raise AgentMemoryConfigError( - f"Failed to load Agent Memory configuration: {exc}" + f"Failed to load Agent Memory configuration for instance '{instance}': {exc}" ) from exc diff --git a/src/sap_cloud_sdk/agent_memory/user-guide.md b/src/sap_cloud_sdk/agent_memory/user-guide.md index ce184bb2..71e18b2e 100644 --- a/src/sap_cloud_sdk/agent_memory/user-guide.md +++ b/src/sap_cloud_sdk/agent_memory/user-guide.md @@ -179,9 +179,10 @@ across create, read, and search calls is the implementer's responsibility. > [!WARNING] > `PROVIDER` strategy provides **no tenant isolation**, the provider token grants access to data in the provider subaccount. Only use this strategy for provider-owned operations (e.g., admin tasks, shared datasets). Never use it to serve subscriber-specific data. - - **Further reading:** N/A + + ## Semantic Search: A Brief Primer Texts with different words — or even different languages — can have the same meaning. diff --git a/tests/agent_memory/integration/conftest.py b/tests/agent_memory/integration/conftest.py index 77e2760a..5bc95b69 100644 --- a/tests/agent_memory/integration/conftest.py +++ b/tests/agent_memory/integration/conftest.py @@ -2,12 +2,21 @@ Set the following environment variables before running integration tests: - CLOUD_SDK_CFG_AGENT_MEMORY_DEFAULT_URL Base URL of the Agent Memory service - CLOUD_SDK_CFG_AGENT_MEMORY_DEFAULT_AUTH_URL OAuth2 authorization server base URL - CLOUD_SDK_CFG_AGENT_MEMORY_DEFAULT_CLIENTID OAuth2 client ID - CLOUD_SDK_CFG_AGENT_MEMORY_DEFAULT_CLIENTSECRET OAuth2 client secret +Provider (default) binding: -Multitenancy: + CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL + CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA + +Subscriber binding (one set per tenant, keyed by subdomain in upper-snake-case): + + CLOUD_SDK_CFG_HANA_AGENT_MEMORY__APPLICATION_URL + CLOUD_SDK_CFG_HANA_AGENT_MEMORY__UAA + + e.g. for tenant "acme-corp": + CLOUD_SDK_CFG_HANA_AGENT_MEMORY_ACME_CORP_APPLICATION_URL + CLOUD_SDK_CFG_HANA_AGENT_MEMORY_ACME_CORP_UAA + +Subscriber tenant name: CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT Subscriber tenant subdomain Required for SUBSCRIBER tests. When absent those tests are skipped. @@ -45,7 +54,16 @@ def agent_memory_client() -> AgentMemoryClient: @pytest.fixture(scope="session") def subscriber_tenant() -> str: - """Return the subscriber tenant subdomain, or skip if not configured.""" + """Return the subscriber tenant subdomain, or skip if not configured. + + On this branch, a separate binding must exist for the tenant subdomain: + /etc/secrets/appfnd/hana-agent-memory// + or environment variables: + CLOUD_SDK_CFG_HANA_AGENT_MEMORY__APPLICATION_URL + CLOUD_SDK_CFG_HANA_AGENT_MEMORY__UAA + """ + from sap_cloud_sdk.agent_memory.config import _load_config_for_instance + env_file = Path(__file__).parents[3] / ".env_integration_tests" if env_file.exists(): load_dotenv(env_file, override=True) @@ -56,4 +74,13 @@ def subscriber_tenant() -> str: "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT not set — " "skipping subscriber tenant tests" ) + + try: + _load_config_for_instance(tenant) + except AgentMemoryConfigError: + pytest.skip( + f"Subscriber binding for tenant '{tenant}' not configured — " + f"skipping subscriber tenant tests" + ) + return tenant diff --git a/tests/agent_memory/unit/test_client.py b/tests/agent_memory/unit/test_client.py index cd6590cb..959ed315 100644 --- a/tests/agent_memory/unit/test_client.py +++ b/tests/agent_memory/unit/test_client.py @@ -61,17 +61,16 @@ def test_uses_provided_config(self): assert isinstance(client, AgentMemoryClient) assert client._transport is not None - def test_subscriber_strategy_loads_default_binding(self, monkeypatch): - """Factory with SUBSCRIBER loads the default binding (native implementation).""" + def test_subscriber_strategy_loads_tenant_binding(self, monkeypatch): + """Factory with SUBSCRIBER loads the tenant binding.""" import json monkeypatch.setenv( - "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", - "http://memory.example.com", + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_ACME_CORP_APPLICATION_URL", + "http://acme.memory.example.com", ) monkeypatch.setenv( - "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", - json.dumps({"url": "http://auth.example.com", "clientid": "c", "clientsecret": "s", - "identityzone": "provider-zone"}), + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_ACME_CORP_UAA", + json.dumps({"url": "http://acme.auth.example.com", "clientid": "c", "clientsecret": "s"}), ) with patch("sap_cloud_sdk.agent_memory.HttpTransport") as MockTransport: MockTransport.return_value = MagicMock(spec=HttpTransport) @@ -80,7 +79,7 @@ def test_subscriber_strategy_loads_default_binding(self, monkeypatch): tenant="acme-corp", ) assert isinstance(client, AgentMemoryClient) - assert client._tenant == "acme-corp" + assert client._transport is not None def test_provider_strategy_loads_default_binding(self, monkeypatch): """Factory with PROVIDER loads the default binding.""" @@ -144,72 +143,11 @@ def test_provider_only_uses_provider_transport(self): provider_transport.post.assert_called_once() -# ── Access strategy ─────────────────────────────────────────────────────────── - - -class TestAccessStrategy: - # ── Init-time validation ────────────────────────────────────────────────── - - def test_subscriber_without_tenant_raises_at_init(self): - """SUBSCRIBER without tenant raises AgentMemoryValidationError at construction.""" - transport = MagicMock(spec=HttpTransport) - with pytest.raises(AgentMemoryValidationError, match="tenant"): - AgentMemoryClient(transport, access_strategy=AccessStrategy.SUBSCRIBER) - - def test_subscriber_with_tenant_constructs_successfully(self): - """SUBSCRIBER with tenant constructs without error and stores tenant.""" - client, _ = _make_subscriber_client("acme") - assert client._tenant == "acme" - - def test_provider_constructs_without_tenant(self): - """PROVIDER constructs without tenant and stores None.""" - client, _ = _make_client() - assert client._tenant is None - - # ── Transport routing ───────────────────────────────────────────────────── - - def test_subscriber_passes_tenant_to_transport(self): - """SUBSCRIBER client passes tenant_subdomain to transport on every call.""" - client, transport = _make_subscriber_client("acme") - transport.post.return_value = { - "id": "m1", - "agentID": "a", - "invokerID": "u", - "content": "x", - } - - client.add_memory("a", "u", "x") - - assert transport.post.call_args[1]["tenant_subdomain"] == "acme" - - def test_provider_passes_none_tenant_to_transport(self): - """PROVIDER client passes tenant_subdomain=None to transport.""" - client, transport = _make_client() - transport.post.return_value = { - "id": "m1", - "agentID": "a", - "invokerID": "u", - "content": "x", - } - - client.add_memory("a", "u", "x") - - assert transport.post.call_args[1]["tenant_subdomain"] is None - - def test_list_memories_passes_tenant_to_transport(self): - """list_memories passes tenant_subdomain from client config.""" - client, transport = _make_subscriber_client("sub") - transport.get.return_value = {"value": []} - - client.list_memories(agent_id="a") - - assert transport.get.call_args[1]["tenant_subdomain"] == "sub" - - # ── Memory CRUD operations ──────────────────────────────────────────────────── class TestMemoryCRUD: + def test_add_memory_posts_correct_payload(self): """add_memory sends required and optional fields in the POST body.""" client, mock_transport = _make_client() @@ -234,10 +172,7 @@ def test_add_memory_with_metadata(self): """Optional metadata is included in the POST body when provided.""" client, mock_transport = _make_client() mock_transport.post.return_value = { - "id": "mem-1", - "agentID": "a", - "invokerID": "u", - "content": "x", + "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "x", } client.add_memory("a", "u", "x", metadata={"key": "val"}) @@ -249,10 +184,7 @@ def test_add_memory_excludes_none_optionals(self): """None-valued optional fields are omitted from the POST body.""" client, mock_transport = _make_client() mock_transport.post.return_value = { - "id": "mem-1", - "agentID": "a", - "invokerID": "u", - "content": "x", + "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "x", } client.add_memory("a", "u", "x") @@ -265,10 +197,7 @@ def test_add_memory_posts_to_memories_endpoint(self): """add_memory sends the POST to the MEMORIES endpoint.""" client, mock_transport = _make_client() mock_transport.post.return_value = { - "id": "mem-1", - "agentID": "a", - "invokerID": "u", - "content": "x", + "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "x", } client.add_memory("a", "u", "x") @@ -280,10 +209,7 @@ def test_get_memory_calls_get_with_memory_id(self): """get_memory constructs the correct path with the memory ID.""" client, mock_transport = _make_client() mock_transport.get.return_value = { - "id": "mem-1", - "agentID": "a", - "invokerID": "u", - "content": "hello", + "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "hello", } memory = client.get_memory("mem-1") @@ -336,6 +262,7 @@ def test_delete_memory_calls_delete(self): class TestListMemories: + def test_returns_list_of_memories(self): """list_memories returns a list of Memory objects.""" client, mock_transport = _make_client() @@ -481,6 +408,7 @@ def test_filter_none_does_not_change_behaviour(self): class TestCountMemories: + def test_returns_count_from_response(self): """count_memories returns the @odata.count value.""" client, mock_transport = _make_client() @@ -526,25 +454,14 @@ def test_returns_zero_when_count_missing(self): class TestSearchMemories: + def test_returns_results_in_api_order(self): """search_memories returns results in the order returned by the API.""" client, mock_transport = _make_client() mock_transport.post.return_value = { "value": [ - { - "id": "m1", - "agentID": "a", - "invokerID": "u", - "content": "first", - "similarity": 0.5, - }, - { - "id": "m2", - "agentID": "a", - "invokerID": "u", - "content": "second", - "similarity": 0.9, - }, + {"id": "m1", "agentID": "a", "invokerID": "u", "content": "first", "similarity": 0.5}, + {"id": "m2", "agentID": "a", "invokerID": "u", "content": "second", "similarity": 0.9}, ] } @@ -597,6 +514,7 @@ def test_uses_default_threshold_and_limit(self): class TestMessageCRUD: + def test_add_message_posts_correct_payload(self): """add_message sends required fields in the POST body.""" client, mock_transport = _make_client() @@ -610,11 +528,7 @@ def test_add_message_posts_correct_payload(self): } message = client.add_message( - "agent-a", - "user-b", - "conv-1", - MessageRole.USER, - "Hello!", + "agent-a", "user-b", "conv-1", MessageRole.USER, "Hello!", ) assert isinstance(message, Message) @@ -631,12 +545,8 @@ def test_add_message_posts_to_messages_endpoint(self): """add_message sends the POST to the MESSAGES endpoint.""" client, mock_transport = _make_client() mock_transport.post.return_value = { - "id": "msg-1", - "agentID": "a", - "invokerID": "u", - "messageGroup": "g", - "role": "USER", - "content": "hi", + "id": "msg-1", "agentID": "a", "invokerID": "u", + "messageGroup": "g", "role": "USER", "content": "hi", } client.add_message("a", "u", "g", MessageRole.USER, "hi") @@ -648,18 +558,12 @@ def test_add_message_with_metadata(self): """Optional metadata is included when provided.""" client, mock_transport = _make_client() mock_transport.post.return_value = { - "id": "msg-1", - "agentID": "a", - "invokerID": "u", - "messageGroup": "g", - "role": "USER", - "content": "hi", + "id": "msg-1", "agentID": "a", "invokerID": "u", + "messageGroup": "g", "role": "USER", "content": "hi", "metadata": {"key": "val"}, } - client.add_message( - "a", "u", "g", MessageRole.USER, "hi", metadata={"key": "val"} - ) + client.add_message("a", "u", "g", MessageRole.USER, "hi", metadata={"key": "val"}) payload = mock_transport.post.call_args[1]["json"] assert payload["metadata"] == {"key": "val"} @@ -668,12 +572,8 @@ def test_add_message_excludes_none_metadata(self): """None-valued metadata is omitted from the POST body.""" client, mock_transport = _make_client() mock_transport.post.return_value = { - "id": "msg-1", - "agentID": "a", - "invokerID": "u", - "messageGroup": "g", - "role": "USER", - "content": "hi", + "id": "msg-1", "agentID": "a", "invokerID": "u", + "messageGroup": "g", "role": "USER", "content": "hi", } client.add_message("a", "u", "g", MessageRole.USER, "hi") @@ -685,12 +585,8 @@ def test_get_message_calls_get_with_message_id(self): """get_message constructs the correct path with the message ID.""" client, mock_transport = _make_client() mock_transport.get.return_value = { - "id": "msg-1", - "agentID": "a", - "invokerID": "u", - "messageGroup": "g", - "role": "USER", - "content": "hi", + "id": "msg-1", "agentID": "a", "invokerID": "u", + "messageGroup": "g", "role": "USER", "content": "hi", } message = client.get_message("msg-1") @@ -714,18 +610,15 @@ def test_delete_message_calls_delete(self): class TestListMessages: + def test_returns_list_of_messages(self): """list_messages returns a list of Message objects.""" client, mock_transport = _make_client() mock_transport.get.return_value = { "value": [ { - "id": "msg-1", - "agentID": "a", - "invokerID": "u", - "messageGroup": "g", - "role": "USER", - "content": "hi", + "id": "msg-1", "agentID": "a", "invokerID": "u", + "messageGroup": "g", "role": "USER", "content": "hi", }, ], } @@ -741,10 +634,8 @@ def test_passes_convenience_filters(self): mock_transport.get.return_value = {"value": []} client.list_messages( - agent_id="a", - invoker_id="u", - message_group="conv-1", - role="USER", + agent_id="a", invoker_id="u", + message_group="conv-1", role="USER", ) params = mock_transport.get.call_args[1]["params"] @@ -881,13 +772,12 @@ def test_filter_none_does_not_change_behaviour(self): class TestRetentionConfig: + def test_get_retention_config(self): """get_retention_config sends GET to the retentionConfig endpoint.""" client, mock_transport = _make_client() mock_transport.get.return_value = { - "id": 1, - "messageDays": 30, - "memoryDays": 90, + "id": 1, "messageDays": 30, "memoryDays": 90, "usageLogDays": 180, "createTimestamp": "2025-01-01T00:00:00Z", "updateTimestamp": "2025-01-02T00:00:00Z", @@ -932,6 +822,7 @@ def test_update_retention_config_excludes_none_fields(self): class TestContextManager: + def test_close_delegates_to_transport(self): """close() delegates to the transport's close method.""" client, mock_transport = _make_client() @@ -955,6 +846,7 @@ def test_context_manager_closes_on_exit(self): class TestMemoryValidation: + def test_add_memory_raises_for_empty_agent_id(self): """add_memory raises AgentMemoryValidationError when agent_id is empty.""" client, _ = _make_client() @@ -1011,6 +903,7 @@ def test_list_memories_raises_for_negative_offset(self): class TestSearchMemoriesValidation: + def test_raises_for_empty_agent_id(self): """search_memories raises AgentMemoryValidationError when agent_id is empty.""" client, _ = _make_client() @@ -1071,6 +964,7 @@ def test_boundary_values_are_accepted(self): class TestMessageValidation: + def test_add_message_raises_for_empty_agent_id(self): """add_message raises AgentMemoryValidationError when agent_id is empty.""" client, _ = _make_client() @@ -1121,6 +1015,7 @@ def test_list_messages_raises_for_negative_offset(self): class TestRetentionConfigValidation: + def test_update_raises_when_no_fields_provided(self): """update_retention_config raises AgentMemoryValidationError when no fields are provided.""" client, _ = _make_client() @@ -1158,6 +1053,7 @@ def test_update_accepts_zero_values(self): class TestFilterDefinitionValidation: + def test_list_memories_raises_for_unsupported_target(self): """list_memories raises AgentMemoryValidationError for an unknown target.""" client, _ = _make_client() diff --git a/tests/agent_memory/unit/test_config.py b/tests/agent_memory/unit/test_config.py index 40e9b2ea..ce297514 100644 --- a/tests/agent_memory/unit/test_config.py +++ b/tests/agent_memory/unit/test_config.py @@ -1,4 +1,4 @@ -"""Unit tests for AgentMemoryConfig, BindingData, and _load_config_from_env.""" +"""Unit tests for AgentMemoryConfig, BindingData, _load_config_from_env, and _load_config_for_instance.""" import json from unittest.mock import patch @@ -8,6 +8,7 @@ from sap_cloud_sdk.agent_memory.config import ( AgentMemoryConfig, BindingData, + _load_config_for_instance, _load_config_from_env, ) from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryConfigError @@ -210,3 +211,77 @@ def test_raises_config_error_when_uaa_json_invalid(self, monkeypatch): with patch("os.stat", side_effect=FileNotFoundError("no mount")): with pytest.raises(AgentMemoryConfigError, match="Failed to parse uaa JSON"): _load_config_from_env() + + +# ── _load_config_for_instance ───────────────────────────────────────────────── + + +def _fill_binding_for_instance(instance_name: str): + """Return a side_effect that fills binding only when instance matches.""" + def _fill(**kwargs): + assert kwargs["instance"] == instance_name + kwargs["target"].application_url = f"https://{instance_name}.memory.example.com" + kwargs["target"].uaa = json.dumps({ + "url": f"https://{instance_name}.auth.example.com", + "clientid": f"{instance_name}-client", + "clientsecret": "secret", + }) + return _fill + + +class TestLoadConfigForInstance: + + def test_loads_named_instance_binding(self): + """Loads config from the specified instance name (not 'default').""" + with patch(_RESOLVER, side_effect=_fill_binding_for_instance("acme-corp")): + config = _load_config_for_instance("acme-corp") + + assert config.base_url == "https://acme-corp.memory.example.com" + assert config.token_url == "https://acme-corp.auth.example.com/oauth/token" + assert config.client_id == "acme-corp-client" + + def test_calls_resolver_with_correct_instance(self): + """Resolver receives the exact instance name passed (not 'default').""" + with patch(_RESOLVER, side_effect=_fill_binding_for_instance("beta-tenant")) as mock_resolver: + _load_config_for_instance("beta-tenant") + + _, kwargs = mock_resolver.call_args + assert kwargs["module"] == "hana-agent-memory" + assert kwargs["instance"] == "beta-tenant" + + def test_default_instance_is_equivalent_to_load_config_from_env(self): + """_load_config_for_instance('default') produces the same result as _load_config_from_env.""" + with patch(_RESOLVER, side_effect=_fill_binding_for_instance("default")): + config_instance = _load_config_for_instance("default") + with patch(_RESOLVER, side_effect=_fill_binding_for_instance("default")): + config_env = _load_config_from_env() + + assert config_instance.base_url == config_env.base_url + assert config_instance.token_url == config_env.token_url + + def test_raises_with_instance_name_in_message_when_binding_missing(self): + """Error message includes the instance name when the binding cannot be loaded.""" + with patch(_RESOLVER, side_effect=RuntimeError("secrets not found")): + with pytest.raises(AgentMemoryConfigError, match="acme-corp"): + _load_config_for_instance("acme-corp") + + def test_loads_from_env_vars_for_named_instance(self, monkeypatch): + """Subscriber binding loaded from env vars keyed by tenant name.""" + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_ACME_CORP_APPLICATION_URL", + "https://acme-corp.memory.example.com", + ) + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_ACME_CORP_UAA", + json.dumps({ + "url": "https://acme-corp.auth.example.com", + "clientid": "acme-client", + "clientsecret": "secret", + }), + ) + + with patch("os.stat", side_effect=FileNotFoundError("no mount")): + config = _load_config_for_instance("acme-corp") + + assert config.base_url == "https://acme-corp.memory.example.com" + assert config.client_id == "acme-client" diff --git a/tests/core/unit/data_anonymization/test_http_transport.py b/tests/core/unit/data_anonymization/test_http_transport.py index b6d9fe72..ec64a7d7 100644 --- a/tests/core/unit/data_anonymization/test_http_transport.py +++ b/tests/core/unit/data_anonymization/test_http_transport.py @@ -279,7 +279,6 @@ def test_resolve_cert_from_destination( transport._tmp_key_file = None cert_path = transport._resolve_cert() - assert isinstance(cert_path, str) assert Path(cert_path).exists() assert "BEGIN RSA PRIVATE KEY" in Path(cert_path).read_text(encoding="utf-8") @@ -329,7 +328,6 @@ def test_resolve_cert_from_destination_with_base64_bundle( transport._tmp_key_file = None cert_path = transport._resolve_cert() - assert isinstance(cert_path, str) assert Path(cert_path).exists() assert "BEGIN CERTIFICATE" in Path(cert_path).read_text(encoding="utf-8") @@ -375,7 +373,7 @@ def test_decode_destination_certificate_content_rejects_missing_key(self) -> Non def test_resolve_cert_without_config_raises(self) -> None: transport = object.__new__(HttpTransport) - transport._config = types.SimpleNamespace( # ty: ignore[invalid-assignment] + transport._config = types.SimpleNamespace( cert=None, key=None, cert_path=None, diff --git a/uv.lock b/uv.lock index 3c3ea997..c9c42952 100644 --- a/uv.lock +++ b/uv.lock @@ -3925,7 +3925,7 @@ wheels = [ [[package]] name = "sap-cloud-sdk" -version = "0.40.0" +version = "0.39.0" source = { editable = "." } dependencies = [ { name = "grpcio" }, From daf77d721a08ac718861e63b74229b30955e3f9e Mon Sep 17 00:00:00 2001 From: Arthur Tonial Date: Fri, 31 Jul 2026 08:20:47 -0300 Subject: [PATCH 2/2] fix: solve type check issues --- tests/core/unit/data_anonymization/test_http_transport.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/core/unit/data_anonymization/test_http_transport.py b/tests/core/unit/data_anonymization/test_http_transport.py index ec64a7d7..87a50c7d 100644 --- a/tests/core/unit/data_anonymization/test_http_transport.py +++ b/tests/core/unit/data_anonymization/test_http_transport.py @@ -280,6 +280,7 @@ def test_resolve_cert_from_destination( cert_path = transport._resolve_cert() + assert isinstance(cert_path, str) assert Path(cert_path).exists() assert "BEGIN RSA PRIVATE KEY" in Path(cert_path).read_text(encoding="utf-8") transport._session = MagicMock() @@ -329,6 +330,7 @@ def test_resolve_cert_from_destination_with_base64_bundle( cert_path = transport._resolve_cert() + assert isinstance(cert_path, str) assert Path(cert_path).exists() assert "BEGIN CERTIFICATE" in Path(cert_path).read_text(encoding="utf-8") transport._session = MagicMock() @@ -373,7 +375,7 @@ def test_decode_destination_certificate_content_rejects_missing_key(self) -> Non def test_resolve_cert_without_config_raises(self) -> None: transport = object.__new__(HttpTransport) - transport._config = types.SimpleNamespace( + transport._config = types.SimpleNamespace( # ty: ignore[invalid-assignment] cert=None, key=None, cert_path=None,