diff --git a/src/sap_cloud_sdk/agent_memory/__init__.py b/src/sap_cloud_sdk/agent_memory/__init__.py index 98195c88..5c520768 100644 --- a/src/sap_cloud_sdk/agent_memory/__init__.py +++ b/src/sap_cloud_sdk/agent_memory/__init__.py @@ -5,9 +5,13 @@ Usage:: - from sap_cloud_sdk.agent_memory import create_client + from sap_cloud_sdk.agent_memory import create_client, AccessStrategy - client = create_client() + # Subscriber tenant — strategy and tenant set once, inherited by all calls + client = create_client( + access_strategy=AccessStrategy.SUBSCRIBER, + tenant="my-tenant-subdomain", + ) memories = client.list_memories(agent_id="my-agent", invoker_id="user-123") """ @@ -24,6 +28,7 @@ AgentMemoryValidationError, ) from sap_cloud_sdk.agent_memory._models import ( + AccessStrategy, Memory, Message, MessageRole, @@ -33,7 +38,12 @@ from sap_cloud_sdk.agent_memory.utils._odata import FilterDefinition -def create_client(*, config: Optional[AgentMemoryConfig] = None) -> AgentMemoryClient: +def create_client( + *, + config: Optional[AgentMemoryConfig] = None, + access_strategy: AccessStrategy = AccessStrategy.SUBSCRIBER, + tenant: Optional[str] = None, +) -> AgentMemoryClient: """Create an :class:`AgentMemoryClient` with automatic credential detection. Args: @@ -41,17 +51,28 @@ def create_client(*, config: Optional[AgentMemoryConfig] = None) -> AgentMemoryC loaded from the mounted volume at ``/etc/secrets/appfnd/hana-agent-memory/default/`` or from ``CLOUD_SDK_CFG_AGENT_MEMORY_DEFAULT_*`` environment variables. + access_strategy: Default tenant access strategy for all client operations. + Defaults to ``SUBSCRIBER``. Individual method calls may override + this value. + tenant: Default subscriber tenant subdomain. Required when + ``access_strategy=SUBSCRIBER``. Individual method calls may + override this value. Returns: A ready-to-use :class:`AgentMemoryClient`. Raises: - AgentMemoryConfigError: If configuration is missing or invalid. + AgentMemoryConfigError: If configuration is missing, invalid, or + ``access_strategy=SUBSCRIBER`` is used without a ``tenant``. """ try: resolved_config = config if config is not None else _load_config_from_env() transport = HttpTransport(resolved_config) - return AgentMemoryClient(transport) + return AgentMemoryClient( + transport, + access_strategy=access_strategy, + tenant=tenant, + ) except AgentMemoryConfigError: raise except Exception as exc: @@ -61,6 +82,7 @@ def create_client(*, config: Optional[AgentMemoryConfig] = None) -> AgentMemoryC __all__ = [ + "AccessStrategy", "AgentMemoryClient", "AgentMemoryConfig", "AgentMemoryError", diff --git a/src/sap_cloud_sdk/agent_memory/_http_transport.py b/src/sap_cloud_sdk/agent_memory/_http_transport.py index e8c80176..9ec4daad 100644 --- a/src/sap_cloud_sdk/agent_memory/_http_transport.py +++ b/src/sap_cloud_sdk/agent_memory/_http_transport.py @@ -1,8 +1,8 @@ """HTTP transport for the Agent Memory service. Handles OAuth2 ``client_credentials`` token acquisition with lazy, -expiry-aware caching. If ``token_url`` is not configured, requests are -sent unauthenticated — expected for local development environments. +expiry-aware caching per tenant subdomain. If ``token_url`` is not configured, +requests are sent unauthenticated — expected for local development environments. """ from __future__ import annotations @@ -31,10 +31,9 @@ class HttpTransport: """Internal HTTP transport for the Agent Memory service. - Manages OAuth2 token lifecycle (lazy acquire + expiry-aware caching) and - attaches the ``Authorization`` header to every request automatically via - ``OAuth2Session``. In no-auth mode (no ``token_url``), a plain - ``requests.Session`` is used instead. + Manages OAuth2 token lifecycle (lazy acquire + expiry-aware caching) per + tenant subdomain and attaches the ``Authorization`` header automatically. + In no-auth mode (no ``token_url``), a plain ``requests.Session`` is used. Args: config: Service configuration. @@ -42,27 +41,34 @@ class HttpTransport: def __init__(self, config: AgentMemoryConfig) -> None: self._config = config - self._oauth: Optional[OAuth2Session] = None + # Keyed by tenant_subdomain (None = provider token) + self._oauth_cache: dict[Optional[str], tuple[OAuth2Session, datetime]] = {} self._plain_session: Optional[requests.Session] = None - self._token_expires_at: Optional[datetime] = None def close(self) -> None: - """Close the underlying HTTP session(s) and release resources.""" - if self._oauth is not None: - self._oauth.close() - self._oauth = None + """Close all underlying HTTP sessions and release resources.""" + for oauth, _ in self._oauth_cache.values(): + oauth.close() + self._oauth_cache.clear() if self._plain_session is not None: self._plain_session.close() self._plain_session = None # ── Public HTTP methods ──────────────────────────────────────────────────── - def get(self, path: str, params: Optional[dict[str, Any]] = None) -> dict[str, Any]: + def get( + self, + path: str, + params: Optional[dict[str, Any]] = None, + *, + tenant_subdomain: Optional[str] = None, + ) -> dict[str, Any]: """Perform a GET request. Args: path: API path (appended to ``base_url``). params: Optional query parameters. + tenant_subdomain: Subscriber tenant subdomain for token derivation. Returns: Parsed JSON response body. @@ -71,14 +77,23 @@ def get(self, path: str, params: Optional[dict[str, Any]] = None) -> dict[str, A AgentMemoryHttpError: On HTTP errors or network failures. AgentMemoryNotFoundError: If the server returns 404. """ - return self._request("GET", path, params=params) + return self._request( + "GET", path, params=params, tenant_subdomain=tenant_subdomain + ) - def post(self, path: str, json: Optional[dict[str, Any]] = None) -> dict[str, Any]: + def post( + self, + path: str, + json: Optional[dict[str, Any]] = None, + *, + tenant_subdomain: Optional[str] = None, + ) -> dict[str, Any]: """Perform a POST request. Args: path: API path (appended to ``base_url``). json: Optional request body dict (serialised to JSON). + tenant_subdomain: Subscriber tenant subdomain for token derivation. Returns: Parsed JSON response body. Returns an empty dict for 204 responses. @@ -87,14 +102,21 @@ def post(self, path: str, json: Optional[dict[str, Any]] = None) -> dict[str, An AgentMemoryHttpError: On HTTP errors or network failures. AgentMemoryNotFoundError: If the server returns 404. """ - return self._request("POST", path, json=json) - - def patch(self, path: str, json: Optional[dict[str, Any]] = None) -> dict[str, Any]: + return self._request("POST", path, json=json, tenant_subdomain=tenant_subdomain) + + def patch( + self, + path: str, + json: Optional[dict[str, Any]] = None, + *, + tenant_subdomain: Optional[str] = None, + ) -> dict[str, Any]: """Perform a PATCH request. Args: path: API path (appended to ``base_url``). json: Optional request body dict (serialised to JSON). + tenant_subdomain: Subscriber tenant subdomain for token derivation. Returns: Parsed JSON response body. Returns an empty dict for 204 responses. @@ -103,46 +125,51 @@ def patch(self, path: str, json: Optional[dict[str, Any]] = None) -> dict[str, A AgentMemoryHttpError: On HTTP errors or network failures. AgentMemoryNotFoundError: If the server returns 404. """ - return self._request("PATCH", path, json=json) + return self._request( + "PATCH", path, json=json, tenant_subdomain=tenant_subdomain + ) - def delete(self, path: str) -> None: + def delete(self, path: str, *, tenant_subdomain: Optional[str] = None) -> None: """Perform a DELETE request. Args: path: API path (appended to ``base_url``). + tenant_subdomain: Subscriber tenant subdomain for token derivation. Raises: AgentMemoryHttpError: On HTTP errors or network failures. AgentMemoryNotFoundError: If the server returns 404. """ - self._request("DELETE", path) + self._request("DELETE", path, tenant_subdomain=tenant_subdomain) # ── Internal helpers ─────────────────────────────────────────────────────── - def _get_session(self) -> requests.Session: - """Return a session ready to make requests. + def _get_session(self, tenant_subdomain: Optional[str]) -> requests.Session: + """Return a session ready to make requests for the given tenant. In no-auth mode, returns a plain ``requests.Session`` (created once). In OAuth2 mode, returns an ``OAuth2Session`` with a valid token, - fetching or refreshing the token if needed. + fetching or refreshing per-tenant as needed. """ if not self._config.token_url: if self._plain_session is None: self._plain_session = requests.Session() return self._plain_session - if ( - self._oauth is not None - and self._token_expires_at is not None - and datetime.now() < self._token_expires_at - ): - return self._oauth + cached = self._oauth_cache.get(tenant_subdomain) + if cached is not None: + oauth, expires_at = cached + if datetime.now() < expires_at: + return oauth - self._oauth = self._fetch_token() - return self._oauth + return self._fetch_token(tenant_subdomain) - def _fetch_token(self) -> OAuth2Session: - """Acquire a new OAuth2 ``client_credentials`` token. + def _fetch_token(self, tenant_subdomain: Optional[str]) -> OAuth2Session: + """Acquire a new OAuth2 ``client_credentials`` token for the given tenant. + + When ``tenant_subdomain`` is provided and ``config.identityzone`` is set, + derives the subscriber token URL by replacing the provider identityzone + in ``token_url`` with ``tenant_subdomain``. Returns: An ``OAuth2Session`` with a valid token attached. @@ -150,11 +177,19 @@ def _fetch_token(self) -> OAuth2Session: Raises: AgentMemoryHttpError: If the token endpoint returns an error or is unreachable. """ + token_url = self._config.token_url + if ( + tenant_subdomain is not None + and self._config.identityzone is not None + and token_url is not None + ): + token_url = token_url.replace(self._config.identityzone, tenant_subdomain) + try: client = BackendApplicationClient(client_id=self._config.client_id) oauth = OAuth2Session(client=client) token = oauth.fetch_token( - token_url=self._config.token_url, + token_url=token_url, client_id=self._config.client_id, client_secret=self._config.client_secret, timeout=self._config.timeout, @@ -163,21 +198,32 @@ def _fetch_token(self) -> OAuth2Session: raise AgentMemoryHttpError(f"Failed to obtain OAuth2 token: {exc}") from exc expires_in: int = token.get("expires_in", 3600) - self._token_expires_at = datetime.now() + timedelta( + expires_at = datetime.now() + timedelta( seconds=expires_in - _TOKEN_EXPIRY_BUFFER_SECONDS ) - if self._oauth is not None: - self._oauth.close() + existing = self._oauth_cache.get(tenant_subdomain) + if existing is not None: + existing[0].close() + + self._oauth_cache[tenant_subdomain] = (oauth, expires_at) logger.debug( - "Obtained new Agent Memory OAuth2 token (expires in %ds)", expires_in + "Obtained new Agent Memory OAuth2 token for tenant=%r (expires in %ds)", + tenant_subdomain, + expires_in, ) return oauth - def _request(self, method: str, path: str, **kwargs: Any) -> dict[str, Any]: + def _request( + self, + method: str, + path: str, + tenant_subdomain: Optional[str] = None, + **kwargs: Any, + ) -> dict[str, Any]: """Execute an HTTP request using the appropriate session.""" - logger.debug("%s %s", method, path) + logger.debug("%s %s (tenant=%r)", method, path, tenant_subdomain) url = f"{self._config.base_url}{path}" if "params" in kwargs: @@ -185,7 +231,7 @@ def _request(self, method: str, path: str, **kwargs: Any) -> dict[str, Any]: if raw_params: url = f"{url}?{urlencode(raw_params, quote_via=quote)}" - session = self._get_session() + session = self._get_session(tenant_subdomain) headers = {"Content-Type": "application/json"} try: diff --git a/src/sap_cloud_sdk/agent_memory/_models.py b/src/sap_cloud_sdk/agent_memory/_models.py index ed41a293..0d9f0862 100644 --- a/src/sap_cloud_sdk/agent_memory/_models.py +++ b/src/sap_cloud_sdk/agent_memory/_models.py @@ -15,6 +15,13 @@ from typing import Any, Optional +class AccessStrategy(str, Enum): + """Access strategy for tenant-scoped Agent Memory operations.""" + + SUBSCRIBER = "SUBSCRIBER" + PROVIDER = "PROVIDER" + + class MessageRole(str, Enum): """Role of the message author.""" diff --git a/src/sap_cloud_sdk/agent_memory/client.py b/src/sap_cloud_sdk/agent_memory/client.py index f12030fb..3d8ee151 100644 --- a/src/sap_cloud_sdk/agent_memory/client.py +++ b/src/sap_cloud_sdk/agent_memory/client.py @@ -9,6 +9,7 @@ from __future__ import annotations +import logging from typing import Any, Optional from sap_cloud_sdk.agent_memory._endpoints import ( @@ -19,6 +20,7 @@ ) from sap_cloud_sdk.agent_memory._http_transport import HttpTransport from sap_cloud_sdk.agent_memory._models import ( + AccessStrategy, Memory, Message, MessageRole, @@ -32,9 +34,13 @@ build_message_filter, extract_value_and_count, ) -from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryValidationError +from sap_cloud_sdk.agent_memory.exceptions import ( + AgentMemoryValidationError, +) from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics +logger = logging.getLogger(__name__) + def _require_non_empty(**fields: str) -> None: """Raise AgentMemoryValidationError if any named field is an empty string.""" @@ -72,10 +78,30 @@ class AgentMemoryClient: Args: transport: HTTP transport layer (injected by ``create_client``). + access_strategy: Tenant access strategy for all operations. + Defaults to ``SUBSCRIBER``. + tenant: Subscriber tenant subdomain. Required when + ``access_strategy=SUBSCRIBER``. """ - def __init__(self, transport: HttpTransport) -> None: + def __init__( + self, + transport: HttpTransport, + *, + access_strategy: AccessStrategy = AccessStrategy.SUBSCRIBER, + tenant: Optional[str] = None, + ) -> None: + if access_strategy is AccessStrategy.SUBSCRIBER and not tenant: + raise AgentMemoryValidationError( + "tenant is required when access_strategy=SUBSCRIBER" + ) + if access_strategy is AccessStrategy.PROVIDER: + logger.warning( + "AccessStrategy.PROVIDER is active: no tenant isolation will be applied. " + "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.""" @@ -105,12 +131,17 @@ def add_memory( invoker_id: Identifier of the user or invoker. content: The memory text content. metadata: Optional metadata dict (Map type in OData). + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: The created :class:`Memory`. Raises: - AgentMemoryValidationError: If any required field is empty. + AgentMemoryValidationError: If any required field is empty or tenant is missing + for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty(agent_id=agent_id, invoker_id=invoker_id, content=content) @@ -121,7 +152,9 @@ def add_memory( } if metadata is not None: payload["metadata"] = metadata - data = self._transport.post(MEMORIES, json=payload) + data = self._transport.post( + MEMORIES, json=payload, tenant_subdomain=self._tenant + ) return Memory.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_GET_MEMORY) @@ -130,17 +163,24 @@ def get_memory(self, memory_id: str) -> Memory: Args: memory_id: The memory identifier (UUID). + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: The :class:`Memory`. Raises: AgentMemoryNotFoundError: If no memory with the given ID exists. - AgentMemoryValidationError: If ``memory_id`` is empty. + AgentMemoryValidationError: If ``memory_id`` is empty or tenant is missing + for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty(memory_id=memory_id) - data = self._transport.get(f"{MEMORIES}({memory_id})") + data = self._transport.get( + f"{MEMORIES}({memory_id})", tenant_subdomain=self._tenant + ) return Memory.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_UPDATE_MEMORY) @@ -157,10 +197,15 @@ def update_memory( memory_id: The memory identifier (UUID). content: New content to set. metadata: New metadata dict to set. + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Raises: AgentMemoryNotFoundError: If no memory with the given ID exists. - AgentMemoryValidationError: If ``memory_id`` is empty or no fields are provided. + AgentMemoryValidationError: If ``memory_id`` is empty, no fields are provided, + or tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty(memory_id=memory_id) @@ -173,7 +218,9 @@ def update_memory( payload["content"] = content if metadata is not None: payload["metadata"] = metadata - self._transport.patch(f"{MEMORIES}({memory_id})", json=payload) + self._transport.patch( + f"{MEMORIES}({memory_id})", json=payload, tenant_subdomain=self._tenant + ) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_DELETE_MEMORY) def delete_memory(self, memory_id: str) -> None: @@ -181,14 +228,21 @@ def delete_memory(self, memory_id: str) -> None: Args: memory_id: The memory identifier (UUID). + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Raises: AgentMemoryNotFoundError: If no memory with the given ID exists. - AgentMemoryValidationError: If ``memory_id`` is empty. + AgentMemoryValidationError: If ``memory_id`` is empty or tenant is missing + for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty(memory_id=memory_id) - self._transport.delete(f"{MEMORIES}({memory_id})") + self._transport.delete( + f"{MEMORIES}({memory_id})", tenant_subdomain=self._tenant + ) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_LIST_MEMORIES) def list_memories( @@ -212,13 +266,17 @@ def list_memories( key-value structured search is not supported. limit: Maximum number of memories to return. Default is ``50``. offset: Number of memories to skip (for pagination). Default is ``0``. + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: List of :class:`Memory` objects. Raises: - AgentMemoryValidationError: If ``limit`` < 1, ``offset`` < 0, or a - filter clause is invalid. + AgentMemoryValidationError: If ``limit`` < 1, ``offset`` < 0, a filter + clause is invalid, or tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ if limit < 1: @@ -236,26 +294,31 @@ def list_memories( top=limit, skip=offset if offset else None, ) - response = self._transport.get(MEMORIES, params=params) + response = self._transport.get( + MEMORIES, params=params, tenant_subdomain=self._tenant + ) items, _ = extract_value_and_count(response) return [Memory.from_dict(item) for item in items] @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_COUNT_MEMORIES) def count_memories( - self, - agent_id: Optional[str] = None, - invoker_id: Optional[str] = None, + self, agent_id: Optional[str] = None, invoker_id: Optional[str] = None ) -> int: """Count memories matching the given filters. Args: agent_id: Filter by agent identifier. invoker_id: Filter by invoker/user identifier. + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: Total number of matching memories. Raises: + AgentMemoryValidationError: If tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ params = build_list_params( @@ -263,7 +326,9 @@ def count_memories( top=0, count=True, ) - response = self._transport.get(MEMORIES, params=params) + response = self._transport.get( + MEMORIES, params=params, tenant_subdomain=self._tenant + ) _, total = extract_value_and_count(response) return total or 0 @@ -284,14 +349,18 @@ def search_memories( query: Natural-language search query (5–5000 characters). threshold: Minimum cosine similarity score (0.0–1.0). Default ``0.6``. limit: Maximum number of results (1–50). Default is ``10``. + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: List of :class:`SearchResult` objects. Raises: - AgentMemoryValidationError: If required fields are empty or parameters are + AgentMemoryValidationError: If required fields are empty, parameters are out of range (``query`` must be 5–5000 chars, ``threshold`` 0.0–1.0, - ``limit`` 1–50). + ``limit`` 1–50), or tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty(agent_id=agent_id, invoker_id=invoker_id, query=query) @@ -310,7 +379,9 @@ def search_memories( "threshold": threshold, "top": limit, } - response = self._transport.post(MEMORY_SEARCH, json=payload) + response = self._transport.post( + MEMORY_SEARCH, json=payload, tenant_subdomain=self._tenant + ) items = response.get("value", []) return [SearchResult.from_dict(item) for item in items] @@ -339,12 +410,17 @@ def add_message( role: Author role (USER, ASSISTANT, SYSTEM, TOOL). content: The message text content. metadata: Optional metadata dict. + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: The created :class:`Message`. Raises: - AgentMemoryValidationError: If any required field is empty. + AgentMemoryValidationError: If any required field is empty or tenant is missing + for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty( @@ -362,7 +438,9 @@ def add_message( } if metadata is not None: payload["metadata"] = metadata - data = self._transport.post(MESSAGES, json=payload) + data = self._transport.post( + MESSAGES, json=payload, tenant_subdomain=self._tenant + ) return Message.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_GET_MESSAGE) @@ -371,17 +449,24 @@ def get_message(self, message_id: str) -> Message: Args: message_id: The message identifier (UUID). + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: The :class:`Message`. Raises: AgentMemoryNotFoundError: If no message with the given ID exists. - AgentMemoryValidationError: If ``message_id`` is empty. + AgentMemoryValidationError: If ``message_id`` is empty or tenant is missing + for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty(message_id=message_id) - data = self._transport.get(f"{MESSAGES}({message_id})") + data = self._transport.get( + f"{MESSAGES}({message_id})", tenant_subdomain=self._tenant + ) return Message.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_DELETE_MESSAGE) @@ -390,14 +475,21 @@ def delete_message(self, message_id: str) -> None: Args: message_id: The message identifier (UUID). + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Raises: AgentMemoryNotFoundError: If no message with the given ID exists. - AgentMemoryValidationError: If ``message_id`` is empty. + AgentMemoryValidationError: If ``message_id`` is empty or tenant is missing + for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ _require_non_empty(message_id=message_id) - self._transport.delete(f"{MESSAGES}({message_id})") + self._transport.delete( + f"{MESSAGES}({message_id})", tenant_subdomain=self._tenant + ) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_LIST_MESSAGES) def list_messages( @@ -425,13 +517,17 @@ def list_messages( key-value structured search is not supported. limit: Maximum number of messages to return. Default is ``50``. offset: Number of messages to skip (for pagination). Default is ``0``. + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Returns: List of :class:`Message` objects. Raises: - AgentMemoryValidationError: If ``limit`` < 1, ``offset`` < 0, or a - filter clause is invalid. + AgentMemoryValidationError: If ``limit`` < 1, ``offset`` < 0, a filter + clause is invalid, or tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ if limit < 1: @@ -451,7 +547,9 @@ def list_messages( top=limit, skip=offset if offset else None, ) - response = self._transport.get(MESSAGES, params=params) + response = self._transport.get( + MESSAGES, params=params, tenant_subdomain=self._tenant + ) items, _ = extract_value_and_count(response) return [Message.from_dict(item) for item in items] @@ -461,13 +559,20 @@ def list_messages( def get_retention_config(self) -> RetentionConfig: """Retrieve the data retention configuration (singleton). + Args: + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. + Returns: The current :class:`RetentionConfig`. Raises: + AgentMemoryValidationError: If tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ - data = self._transport.get(RETENTION_CONFIG) + data = self._transport.get(RETENTION_CONFIG, tenant_subdomain=self._tenant) return RetentionConfig.from_dict(data) @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_UPDATE_RETENTION_CONFIG) @@ -488,10 +593,14 @@ def update_retention_config( message_days: How long to keep messages (days). memory_days: How long to keep memories without access (days). usage_log_days: How long to keep access and search logs (days). + access_strategy: Tenant access strategy. Overrides the client default when + provided. Falls back to the default set on :func:`create_client`. + tenant: Subscriber tenant subdomain. Overrides the client default when + provided. Required (at call or client level) when strategy is ``SUBSCRIBER``. Raises: - AgentMemoryValidationError: If no fields are provided, or any provided - value is negative. + AgentMemoryValidationError: If no fields are provided, any provided value is + negative, or tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ if message_days is None and memory_days is None and usage_log_days is None: @@ -513,4 +622,6 @@ 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) + self._transport.patch( + RETENTION_CONFIG, json=payload, tenant_subdomain=self._tenant + ) diff --git a/src/sap_cloud_sdk/agent_memory/config.py b/src/sap_cloud_sdk/agent_memory/config.py index e2cbdfa7..7fe97dfb 100644 --- a/src/sap_cloud_sdk/agent_memory/config.py +++ b/src/sap_cloud_sdk/agent_memory/config.py @@ -35,6 +35,9 @@ class AgentMemoryConfig: requests are sent without authentication (useful for local development). client_id: The OAuth2 client ID. Optional. client_secret: The OAuth2 client secret. Optional. + identityzone: The provider tenant identity zone subdomain. Required when using + ``SUBSCRIBER`` access strategy so the subscriber token URL + can be derived by replacing this value in ``token_url``. timeout: Timeout in seconds for HTTP requests. Default is 30.0. Example — deployed BTP service:: @@ -44,6 +47,7 @@ class AgentMemoryConfig: token_url="https://.authentication./oauth/token", client_id="", client_secret="", + identityzone="", ) Example — local development (no auth):: @@ -55,6 +59,7 @@ class AgentMemoryConfig: token_url: Optional[str] = None client_id: Optional[str] = None client_secret: Optional[str] = None + identityzone: Optional[str] = None timeout: float = 30.0 def __post_init__(self) -> None: @@ -72,6 +77,10 @@ def __post_init__(self) -> None: raise AgentMemoryConfigError( "client_secret must be a non-empty string when provided" ) + if self.identityzone is not None and not self.identityzone: + raise AgentMemoryConfigError( + "identityzone must be a non-empty string when provided" + ) @dataclass @@ -108,6 +117,7 @@ def extract_config(self) -> AgentMemoryConfig: token_url=uaa_data["url"].rstrip("/") + "/oauth/token", client_id=uaa_data["clientid"], client_secret=uaa_data["clientsecret"], + identityzone=uaa_data.get("identityzone"), ) except KeyError as e: raise AgentMemoryConfigError(f"Missing required field in uaa JSON: {e}") diff --git a/src/sap_cloud_sdk/agent_memory/user-guide.md b/src/sap_cloud_sdk/agent_memory/user-guide.md index 7eb4d56a..8d78423b 100644 --- a/src/sap_cloud_sdk/agent_memory/user-guide.md +++ b/src/sap_cloud_sdk/agent_memory/user-guide.md @@ -22,6 +22,11 @@ plain text, and the service makes it searchable by meaning. - [Core Concepts](#core-concepts) - [`agent_id`](#agent_id) - [`invoker_id`](#invoker_id) + - [Multitenancy](#multitenancy) + - [AccessStrategy](#accessstrategy) + - [Configuring at client level](#configuring-at-client-level) + - [SUBSCRIBER\_ONLY (default)](#subscriber_only-default) + - [PROVIDER\_ONLY](#provider_only) - [Semantic Search: A Brief Primer](#semantic-search-a-brief-primer) - [Memories](#memories) - [Create a Memory](#create-a-memory) @@ -164,6 +169,88 @@ the application's auth system. Memories and messages are scoped to the combinati Neither value is validated by the service — they are free-form strings. Consistent use across create, read, and search calls is the implementer's responsibility. +## Multitenancy + +The Agent Memory service runs in a multi-tenant BTP environment. By default, every API +call uses a **subscriber-scoped token** — meaning data is isolated to the subscriber tenant +that your application serves. You control this behaviour with the `access_strategy` and +`tenant` keyword arguments available on every client method. + +### AccessStrategy + +```python +from sap_cloud_sdk.agent_memory import AccessStrategy +``` + +| Value | Description | +| --------------------------- | ------------------------------------------------------------------------------------------------------------- | +| `SUBSCRIBER` (default) | Reads and writes against the subscriber tenant. Requires `tenant`. | +| `PROVIDER` | Reads and writes against the provider tenant. No `tenant` needed. Caution: this provides no tenant isolation. | + +### Configuring at client level + +Pass `access_strategy` and `tenant` to `create_client()` to set defaults for the entire +client instance. Every method call then inherits them, so you do not need to repeat them +on each operation. + +```python +from sap_cloud_sdk.agent_memory import create_client, AccessStrategy + +# Tenant set once — all calls below use it automatically +client = create_client( + access_strategy=AccessStrategy.SUBSCRIBER, + tenant="acme-corp", +) + +memories = client.list_memories(agent_id="hr-assistant", invoker_id="user-42") +count = client.count_memories(agent_id="hr-assistant") +``` + +A per-call value overrides the client default for that single call: + +```python +# All calls use SUBSCRIBER / "acme-corp" except this one +provider_memories = client.list_memories( + agent_id="hr-assistant", + invoker_id="user-42", + access_strategy=AccessStrategy.PROVIDER, # overrides for this call only +) +``` + +### SUBSCRIBER (default) + +Pass the subscriber tenant subdomain via the `tenant` argument. Omitting `tenant` when +the strategy is `SUBSCRIBER` raises `AgentMemoryValidationError`. + +```python +memories = client.list_memories( + agent_id="hr-assistant", + invoker_id="user-42", + access_strategy=AccessStrategy.SUBSCRIBER, + tenant="acme-corp", # subscriber subdomain +) +``` + +The subscriber token URL is derived by replacing the provider's `identityzone` segment +in the configured `token_url` with the `tenant` value. This requires `identityzone` to +be present in the service binding's UAA JSON (standard XSUAA field) or set explicitly in +`AgentMemoryConfig`. + +### PROVIDER + +No `tenant` argument is needed. All calls use the provider token. + +```python +memories = client.list_memories( + agent_id="hr-assistant", + invoker_id="user-42", + access_strategy=AccessStrategy.PROVIDER, +) +``` + +> [!WARNING] +> `PROVIDER` provides **no tenant isolation** — the provider token grants access to data across all subscriber tenants Only use this strategy for provider-owned operations (e.g., admin tasks, shared datasets). Never use it to serve subscriber-specific data. + ## Semantic Search: A Brief Primer Texts with different words — or even different languages — can have the same meaning. @@ -501,9 +588,10 @@ See the [Content and metadata filtering](#content-and-metadata-filtering) note u ### Enums -| Enum | Values | -| ------------- | ------------------------------------- | -| `MessageRole` | `USER`, `ASSISTANT`, `SYSTEM`, `TOOL` | +| Enum | Values | +| ---------------- | -------------------------------------------- | +| `MessageRole` | `USER`, `ASSISTANT`, `SYSTEM`, `TOOL` | +| `AccessStrategy` | `SUBSCRIBER` (default), `PROVIDER` | All models expose a `to_dict()` method that returns a plain dict for logging or forwarding. diff --git a/tests/agent_memory/integration/agentmemory.feature b/tests/agent_memory/integration/agentmemory.feature index e6804f2d..7321c5de 100644 --- a/tests/agent_memory/integration/agentmemory.feature +++ b/tests/agent_memory/integration/agentmemory.feature @@ -89,3 +89,93 @@ Feature: Agent Memory Service Integration (v1 API) Given a message exists with agent "test-agent" invoker "test-user" group "conv-filter" role "USER" content "filter-test-message" and metadata "filter-marker" When I list messages filtered by metadata containing "filter-marker" Then the result should contain the message with content "filter-test-message" + + # ── Subscriber access ──────────────────────────────────────────────────────── + + # ──── Memory CRUD ───────────────────────────────────────────────────────────── + + Scenario: Create a new memory using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + When I create a memory with agent "test-agent" and invoker "test-user" and content "User prefers dark mode" + Then the memory should have a non-empty id + And the memory should have agent_id "test-agent" + And the memory should have invoker_id "test-user" + And the memory should have content "User prefers dark mode" + + Scenario: Get a memory using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a memory exists with agent "test-agent" and invoker "test-user" and content "Test memory" + When I get the memory by id + Then the returned memory should match the created memory + + Scenario: Update memory content using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a memory exists with agent "test-agent" and invoker "test-user" and content "Original content" + When I update the memory content to "Updated content" + Then the memory should have content "Updated content" + + Scenario: List memories using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a memory exists with agent "test-agent" and invoker "test-user" and content "Listed memory" + When I list memories filtered by agent "test-agent" + Then the result should contain at least one memory + And the total count should be a positive number + + Scenario: Delete a memory using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a memory exists with agent "test-agent" and invoker "test-user" and content "To be deleted" + When I delete the memory + Then the memory should no longer exist + + # ──── Memory search ─────────────────────────────────────────────────────────── + + Scenario: Search memories using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a memory exists with agent "test-agent" and invoker "test-user" and content "The user loves dark mode and dark themes" + When I search for memories with query "dark mode preference" + Then the search result should contain at least one result + And each result should have a non-empty content + + # ──── Message CRUD ──────────────────────────────────────────────────────────── + + Scenario: Create and get a message using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + When I create a message with agent "test-agent" invoker "test-user" group "conv-1" role "USER" content "Hello!" + Then the message should have a non-empty id + And the message should have role "USER" + And the message should have content "Hello!" + + Scenario: List messages using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a message exists with agent "test-agent" invoker "test-user" group "conv-list" role "USER" content "Listed message" + When I list messages filtered by agent "test-agent" and group "conv-list" + Then the result should contain at least one message + And the total count should be a positive number + + Scenario: Delete a message using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a message exists with agent "test-agent" invoker "test-user" group "conv-del" role "USER" content "To be deleted" + When I delete the message + Then the message should no longer exist + + # ──── Bulk / utility operations ─────────────────────────────────────────────── + + Scenario: Count memories using SUBSCRIBER access strategy + Given I use the configured subscriber tenant + And a memory exists with agent "test-agent" and invoker "test-user" and content "Count test memory" + When I count memories for agent "test-agent" and invoker "test-user" + Then the memory count should be a positive number + + # ──── Filter ────────────────────────────────────────────────────────────────── + + Scenario: Filter subscriber memories by content substring + Given I use the configured subscriber tenant + And a memory exists with agent "test-agent" and invoker "test-user" and content "The user prefers dark mode" + When I list memories filtered by content containing "dark mode" + Then the result should contain the memory with content "The user prefers dark mode" + + Scenario: Filter subscriber messages by metadata substring + Given I use the configured subscriber tenant + And a message exists with agent "test-agent" invoker "test-user" group "conv-filter" role "USER" content "filter-test-message" and metadata "filter-marker" + When I list messages filtered by metadata containing "filter-marker" + Then the result should contain the message with content "filter-test-message" diff --git a/tests/agent_memory/integration/conftest.py b/tests/agent_memory/integration/conftest.py index 9b26d5c0..77e2760a 100644 --- a/tests/agent_memory/integration/conftest.py +++ b/tests/agent_memory/integration/conftest.py @@ -6,25 +6,54 @@ 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 + +Multitenancy: + + CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT Subscriber tenant subdomain + Required for SUBSCRIBER tests. When absent those tests are skipped. """ +import os from pathlib import Path import pytest from dotenv import load_dotenv -from sap_cloud_sdk.agent_memory import create_client +from sap_cloud_sdk.agent_memory import AccessStrategy, create_client from sap_cloud_sdk.agent_memory.client import AgentMemoryClient +from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryConfigError @pytest.fixture(scope="session") def agent_memory_client() -> AgentMemoryClient: - """Create a real AgentMemoryClient from environment variables.""" + """Create a real AgentMemoryClient from environment variables. + + Uses PROVIDER as the default strategy — individual BDD steps override + this per-call to exercise both PROVIDER and SUBSCRIBER scenarios. + """ env_file = Path(__file__).parents[3] / ".env_integration_tests" if env_file.exists(): load_dotenv(env_file, override=True) try: - return create_client() + return create_client(access_strategy=AccessStrategy.PROVIDER) + except AgentMemoryConfigError as e: + pytest.skip(f"Agent Memory credentials not configured — skipping integration tests: {e}") except Exception as e: pytest.fail(f"Failed to create Agent Memory client for integration tests: {e}") + + +@pytest.fixture(scope="session") +def subscriber_tenant() -> str: + """Return the subscriber tenant subdomain, or skip if not configured.""" + env_file = Path(__file__).parents[3] / ".env_integration_tests" + if env_file.exists(): + load_dotenv(env_file, override=True) + + tenant = os.environ.get("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT", "") + if not tenant: + pytest.skip( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT not set — " + "skipping subscriber tenant tests" + ) + return tenant diff --git a/tests/agent_memory/integration/test_agentmemory_bdd.py b/tests/agent_memory/integration/test_agentmemory_bdd.py index 41460b68..b0caab21 100644 --- a/tests/agent_memory/integration/test_agentmemory_bdd.py +++ b/tests/agent_memory/integration/test_agentmemory_bdd.py @@ -14,13 +14,15 @@ """ import pytest -from pytest_bdd import given, scenario, then, when +from pytest_bdd import given, parsers, scenario, then, when +from sap_cloud_sdk.agent_memory import AccessStrategy, FilterDefinition, MessageRole from sap_cloud_sdk.agent_memory.client import AgentMemoryClient -from sap_cloud_sdk.agent_memory import MessageRole # -- Scenarios ----------------------------------------------------------------- +# ── Provider scenarios ──────────────────────────────────────────────────────── + @scenario("agentmemory.feature", "Create a new memory") def test_add_memory(): @@ -92,12 +94,78 @@ def test_filter_messages_by_metadata(): pass +# ── Subscriber scenarios ────────────────────────────────────────────────────── + + +@scenario("agentmemory.feature", "Create a new memory using SUBSCRIBER access strategy") +def test_add_memory_subscriber(): + pass + + +@scenario("agentmemory.feature", "Get a memory using SUBSCRIBER access strategy") +def test_get_memory_subscriber(): + pass + + +@scenario("agentmemory.feature", "Update memory content using SUBSCRIBER access strategy") +def test_update_memory_subscriber(): + pass + + +@scenario("agentmemory.feature", "List memories using SUBSCRIBER access strategy") +def test_list_memories_subscriber(): + pass + + +@scenario("agentmemory.feature", "Delete a memory using SUBSCRIBER access strategy") +def test_delete_memory_subscriber(): + pass + + +@scenario("agentmemory.feature", "Search memories using SUBSCRIBER access strategy") +def test_search_memories_subscriber(): + pass + + +@scenario("agentmemory.feature", "Create and get a message using SUBSCRIBER access strategy") +def test_add_message_subscriber(): + pass + + +@scenario("agentmemory.feature", "List messages using SUBSCRIBER access strategy") +def test_list_messages_subscriber(): + pass + + +@scenario("agentmemory.feature", "Delete a message using SUBSCRIBER access strategy") +def test_delete_message_subscriber(): + pass + + +@scenario("agentmemory.feature", "Count memories using SUBSCRIBER access strategy") +def test_count_memories_subscriber(): + pass + + +@scenario("agentmemory.feature", "Filter subscriber memories by content substring") +def test_filter_memories_by_content_subscriber(): + pass + + +@scenario("agentmemory.feature", "Filter subscriber messages by metadata substring") +def test_filter_messages_by_metadata_subscriber(): + pass + + # -- Fixtures / state --------------------------------------------------------- @pytest.fixture def context(): - return {} + return { + "access_strategy": AccessStrategy.PROVIDER, + "tenant": None, + } # -- Given steps --------------------------------------------------------------- @@ -108,91 +176,46 @@ def configured_client(context, agent_memory_client): context["client"] = agent_memory_client -@given( - 'a memory exists with agent "test-agent" and invoker "test-user" and content "Test memory"' -) -def memory_exists_test(context, agent_memory_client): - context["client"] = agent_memory_client - context["memory"] = agent_memory_client.add_memory( - "test-agent", - "test-user", - "Test memory", - ) +@given("I use the configured subscriber tenant") +def use_configured_subscriber_tenant(context, subscriber_tenant): + context["access_strategy"] = AccessStrategy.SUBSCRIBER + context["tenant"] = subscriber_tenant @given( - 'a memory exists with agent "test-agent" and invoker "test-user" and content "Original content"' -) -def memory_exists_original(context, agent_memory_client): - context["client"] = agent_memory_client - context["memory"] = agent_memory_client.add_memory( - "test-agent", - "test-user", - "Original content", + parsers.parse( + 'a memory exists with agent "{agent_id}" and invoker "{invoker_id}" and content "{content}"' ) - - -@given( - 'a memory exists with agent "test-agent" and invoker "test-user" and content "Listed memory"' -) -def memory_exists_listed(context, agent_memory_client): - context["client"] = agent_memory_client - context["memory"] = agent_memory_client.add_memory( - "test-agent", - "test-user", - "Listed memory", - ) - - -@given( - 'a memory exists with agent "test-agent" and invoker "test-user" and content "To be deleted"' ) -def memory_exists_delete(context, agent_memory_client): +def memory_exists(context, agent_memory_client, agent_id, invoker_id, content): context["client"] = agent_memory_client context["memory"] = agent_memory_client.add_memory( - "test-agent", - "test-user", - "To be deleted", + agent_id, invoker_id, content, ) @given( - 'a memory exists with agent "test-agent" and invoker "test-user" and content "The user loves dark mode and dark themes"' -) -def memory_exists_search(context, agent_memory_client): - context["client"] = agent_memory_client - context["memory"] = agent_memory_client.add_memory( - "test-agent", - "test-user", - "The user loves dark mode and dark themes", + parsers.parse( + 'a message exists with agent "{agent_id}" invoker "{invoker_id}" group "{group}" role "{role}" content "{content}"' ) - - -@given( - 'a message exists with agent "test-agent" invoker "test-user" group "conv-list" role "USER" content "Listed message"' ) -def message_exists_list(context, agent_memory_client): +def message_exists(context, agent_memory_client, agent_id, invoker_id, group, role, content): context["client"] = agent_memory_client context["message"] = agent_memory_client.add_message( - "test-agent", - "test-user", - "conv-list", - "USER", - "Listed message", + agent_id, invoker_id, group, role, content, ) @given( - 'a message exists with agent "test-agent" invoker "test-user" group "conv-del" role "USER" content "To be deleted"' + parsers.parse( + 'a message exists with agent "{agent_id}" invoker "{invoker_id}" group "{group}" role "{role}" content "{content}" and metadata "{metadata_value}"' + ) ) -def message_exists_delete(context, agent_memory_client): +def message_exists_with_metadata(context, agent_memory_client, agent_id, invoker_id, group, role, content, metadata_value): context["client"] = agent_memory_client context["message"] = agent_memory_client.add_message( - "test-agent", - "test-user", - "conv-del", - "USER", - "To be deleted", + agent_id, invoker_id, group, role, content, + metadata={"tag": metadata_value}, ) @@ -200,76 +223,90 @@ def message_exists_delete(context, agent_memory_client): @when( - 'I create a memory with agent "test-agent" and invoker "test-user" and content "User prefers dark mode"' + parsers.parse( + 'I create a memory with agent "{agent_id}" and invoker "{invoker_id}" and content "{content}"' + ) ) -def add_memory(context): +def add_memory(context, agent_id, invoker_id, content): client: AgentMemoryClient = context["client"] context["memory"] = client.add_memory( - "test-agent", - "test-user", - "User prefers dark mode", + agent_id, invoker_id, content, ) @when("I get the memory by id") def get_memory(context): client: AgentMemoryClient = context["client"] - context["fetched_memory"] = client.get_memory(context["memory"].id) + context["fetched_memory"] = client.get_memory( + context["memory"].id, + ) -@when('I update the memory content to "Updated content"') -def update_memory(context): +@when(parsers.parse('I update the memory content to "{content}"')) +def update_memory(context, content): client: AgentMemoryClient = context["client"] - client.update_memory(context["memory"].id, content="Updated content") - context["memory"] = client.get_memory(context["memory"].id) + client.update_memory( + context["memory"].id, content=content, + ) + context["memory"] = client.get_memory( + context["memory"].id, + ) -@when('I list memories filtered by agent "test-agent"') -def list_memories(context): +@when(parsers.parse('I list memories filtered by agent "{agent_id}"')) +def list_memories(context, agent_id): client: AgentMemoryClient = context["client"] - context["memories"] = client.list_memories(agent_id="test-agent") - context["total"] = client.count_memories(agent_id="test-agent") + context["memories"] = client.list_memories( + agent_id=agent_id, + ) + context["total"] = client.count_memories( + agent_id=agent_id, + ) @when("I delete the memory") def delete_memory(context): client: AgentMemoryClient = context["client"] - client.delete_memory(context["memory"].id) + client.delete_memory( + context["memory"].id, + ) context["deleted_memory_id"] = context["memory"].id -@when('I search for memories with query "dark mode preference"') -def search_memories(context): +@when(parsers.parse('I search for memories with query "{query}"')) +def search_memories(context, query): client: AgentMemoryClient = context["client"] context["search_results"] = client.search_memories( agent_id="test-agent", invoker_id="test-user", - query="dark mode preference", + query=query, threshold=0.5, limit=10, ) @when( - 'I create a message with agent "test-agent" invoker "test-user" group "conv-1" role "USER" content "Hello!"' + parsers.parse( + 'I create a message with agent "{agent_id}" invoker "{invoker_id}" group "{group}" role "{role}" content "{content}"' + ) ) -def add_message(context): +def add_message(context, agent_id, invoker_id, group, role, content): client: AgentMemoryClient = context["client"] context["message"] = client.add_message( - "test-agent", - "test-user", - "conv-1", - MessageRole.USER, - "Hello!", + agent_id, invoker_id, group, MessageRole(role), content, ) -@when('I list messages filtered by agent "test-agent" and group "conv-list"') -def list_messages(context): +@when( + parsers.parse( + 'I list messages filtered by agent "{agent_id}" and group "{group}"' + ) +) +def list_messages(context, agent_id, group): client: AgentMemoryClient = context["client"] context["messages"] = client.list_messages( - agent_id="test-agent", - message_group="conv-list", + agent_id=agent_id, + message_group=group, ) context["total"] = len(context["messages"]) @@ -277,10 +314,63 @@ def list_messages(context): @when("I delete the message") def delete_message(context): client: AgentMemoryClient = context["client"] - client.delete_message(context["message"].id) + client.delete_message( + context["message"].id, + ) context["deleted_message_id"] = context["message"].id +@when("I get the retention config") +def get_retention_config(context): + client: AgentMemoryClient = context["client"] + context["retention_config"] = client.get_retention_config( + ) + + +@when("I update the retention config with message_days 30 and memory_days 90") +def update_retention_config(context): + client: AgentMemoryClient = context["client"] + client.update_retention_config( + message_days=30, memory_days=90, + ) + context["retention_config"] = client.get_retention_config( + ) + + +@when( + parsers.parse( + 'I count memories for agent "{agent_id}" and invoker "{invoker_id}"' + ) +) +def count_memories(context, agent_id, invoker_id): + client: AgentMemoryClient = context["client"] + context["memory_count"] = client.count_memories( + agent_id=agent_id, + invoker_id=invoker_id, + ) + + +@when(parsers.parse('I list memories filtered by content containing "{substring}"')) +def list_memories_by_content(context, substring): + client: AgentMemoryClient = context["client"] + context["memories"] = client.list_memories( + agent_id="test-agent", + invoker_id="test-user", + filters=[FilterDefinition(target="content", contains=substring)], + ) + + +@when(parsers.parse('I list messages filtered by metadata containing "{substring}"')) +def list_messages_by_metadata(context, substring): + client: AgentMemoryClient = context["client"] + context["messages"] = client.list_messages( + agent_id="test-agent", + invoker_id="test-user", + message_group="conv-filter", + filters=[FilterDefinition(target="metadata", contains=substring)], + ) + + # -- Then steps ---------------------------------------------------------------- @@ -289,24 +379,19 @@ def check_memory_id(context): assert context["memory"].id != "" -@then('the memory should have agent_id "test-agent"') -def check_memory_agent_id(context): - assert context["memory"].agent_id == "test-agent" +@then(parsers.parse('the memory should have agent_id "{agent_id}"')) +def check_memory_agent_id(context, agent_id): + assert context["memory"].agent_id == agent_id -@then('the memory should have invoker_id "test-user"') -def check_memory_invoker_id(context): - assert context["memory"].invoker_id == "test-user" +@then(parsers.parse('the memory should have invoker_id "{invoker_id}"')) +def check_memory_invoker_id(context, invoker_id): + assert context["memory"].invoker_id == invoker_id -@then('the memory should have content "User prefers dark mode"') -def check_memory_content_dark(context): - assert context["memory"].content == "User prefers dark mode" - - -@then('the memory should have content "Updated content"') -def check_memory_content_updated(context): - assert context["memory"].content == "Updated content" +@then(parsers.parse('the memory should have content "{content}"')) +def check_memory_content(context, content): + assert context["memory"].content == content @then("the returned memory should match the created memory") @@ -332,7 +417,9 @@ def check_memory_deleted(context): client: AgentMemoryClient = context["client"] with pytest.raises(AgentMemoryNotFoundError): - client.get_memory(context["deleted_memory_id"]) + client.get_memory( + context["deleted_memory_id"], + ) @then("the search result should contain at least one result") @@ -356,9 +443,9 @@ def check_message_role(context): assert context["message"].role == "USER" -@then('the message should have content "Hello!"') -def check_message_content(context): - assert context["message"].content == "Hello!" +@then(parsers.parse('the message should have content "{content}"')) +def check_message_content(context, content): + assert context["message"].content == content @then("the result should contain at least one message") @@ -372,23 +459,9 @@ def check_message_deleted(context): client: AgentMemoryClient = context["client"] with pytest.raises(AgentMemoryNotFoundError): - client.get_message(context["deleted_message_id"]) - - -# -- Admin: Retention Config steps --------------------------------------------- - - -@when("I get the retention config") -def get_retention_config(context): - client: AgentMemoryClient = context["client"] - context["retention_config"] = client.get_retention_config() - - -@when("I update the retention config with message_days 30 and memory_days 90") -def update_retention_config(context): - client: AgentMemoryClient = context["client"] - client.update_retention_config(message_days=30, memory_days=90) - context["retention_config"] = client.get_retention_config() + client.get_message( + context["deleted_message_id"], + ) @then("the retention config should have a non-empty id") @@ -406,97 +479,18 @@ def check_retention_memory_days(context): assert context["retention_config"].memory_days == 90 -# -- Bulk / utility steps ------------------------------------------------------- - - -@given( - 'a memory exists with agent "test-agent" and invoker "test-user" and content "Count test memory"' -) -def memory_exists_count(context, agent_memory_client): - context["client"] = agent_memory_client - context["memory"] = agent_memory_client.add_memory( - "test-agent", - "test-user", - "Count test memory", - ) - - -@when('I count memories for agent "test-agent" and invoker "test-user"') -def count_memories(context): - client: AgentMemoryClient = context["client"] - context["memory_count"] = client.count_memories( - agent_id="test-agent", - invoker_id="test-user", - ) - - @then("the memory count should be a positive number") def check_memory_count_positive(context): assert context["memory_count"] >= 1 -# -- Filter steps --------------------------------------------------------------- - - -@given( - 'a memory exists with agent "test-agent" and invoker "test-user" and content "The user prefers dark mode"' -) -def memory_exists_dark_mode(context, agent_memory_client): - context["client"] = agent_memory_client - context["memory"] = agent_memory_client.add_memory( - "test-agent", - "test-user", - "The user prefers dark mode", - ) - - -@given( - 'a message exists with agent "test-agent" invoker "test-user" group "conv-filter" role "USER" content "filter-test-message" and metadata "filter-marker"' -) -def message_exists_filter(context, agent_memory_client): - context["client"] = agent_memory_client - context["message"] = agent_memory_client.add_message( - "test-agent", - "test-user", - "conv-filter", - "USER", - "filter-test-message", - metadata={"tag": "filter-marker"}, - ) - - -@when('I list memories filtered by content containing "dark mode"') -def list_memories_by_content(context): - from sap_cloud_sdk.agent_memory import FilterDefinition - - client: AgentMemoryClient = context["client"] - context["memories"] = client.list_memories( - agent_id="test-agent", - invoker_id="test-user", - filters=[FilterDefinition(target="content", contains="dark mode")], - ) - - -@when('I list messages filtered by metadata containing "filter-marker"') -def list_messages_by_metadata(context): - from sap_cloud_sdk.agent_memory import FilterDefinition - - client: AgentMemoryClient = context["client"] - context["messages"] = client.list_messages( - agent_id="test-agent", - invoker_id="test-user", - message_group="conv-filter", - filters=[FilterDefinition(target="metadata", contains="filter-marker")], - ) - - -@then('the result should contain the memory with content "The user prefers dark mode"') -def check_memory_content_in_results(context): +@then(parsers.parse('the result should contain the memory with content "{content}"')) +def check_memory_content_in_results(context, content): contents = [m.content for m in context["memories"]] - assert "The user prefers dark mode" in contents + assert content in contents -@then('the result should contain the message with content "filter-test-message"') -def check_message_content_in_results(context): +@then(parsers.parse('the result should contain the message with content "{content}"')) +def check_message_content_in_results(context, content): contents = [m.content for m in context["messages"]] - assert "filter-test-message" in contents + assert content in contents diff --git a/tests/agent_memory/unit/test_client.py b/tests/agent_memory/unit/test_client.py index b3dbbdd3..f027745a 100644 --- a/tests/agent_memory/unit/test_client.py +++ b/tests/agent_memory/unit/test_client.py @@ -11,6 +11,7 @@ ) from sap_cloud_sdk.agent_memory._http_transport import HttpTransport from sap_cloud_sdk.agent_memory._models import ( + AccessStrategy, Memory, Message, MessageRole, @@ -24,9 +25,20 @@ def _make_client() -> tuple[AgentMemoryClient, MagicMock]: - """Return an AgentMemoryClient with a mocked transport layer.""" + """Return an AgentMemoryClient with a mocked transport and PROVIDER default.""" transport = MagicMock(spec=HttpTransport) - client = AgentMemoryClient(transport) + client = AgentMemoryClient(transport, access_strategy=AccessStrategy.PROVIDER) + return client, transport + + +def _make_subscriber_client( + tenant: str = "default-sub", +) -> tuple[AgentMemoryClient, MagicMock]: + """Return a client with SUBSCRIBER default and the given tenant.""" + transport = MagicMock(spec=HttpTransport) + client = AgentMemoryClient( + transport, access_strategy=AccessStrategy.SUBSCRIBER, tenant=tenant + ) return client, transport @@ -34,35 +46,119 @@ def _make_client() -> tuple[AgentMemoryClient, MagicMock]: class TestCreateClient: - - def test_uses_provided_config(self): - """Factory accepts an explicit config object.""" + def test_uses_provided_config_with_provider_strategy(self): + """Factory with explicit config and PROVIDER constructs successfully.""" config = AgentMemoryConfig(base_url="http://localhost:3000") with patch("sap_cloud_sdk.agent_memory.HttpTransport") as MockTransport: MockTransport.return_value = MagicMock(spec=HttpTransport) - client = create_client(config=config) + client = create_client( + config=config, access_strategy=AccessStrategy.PROVIDER + ) assert isinstance(client, AgentMemoryClient) + assert client._tenant is None + + def test_uses_provided_config_with_subscriber_strategy(self): + """Factory with explicit config and SUBSCRIBER stores tenant.""" + config = AgentMemoryConfig(base_url="http://localhost:3000") + with patch("sap_cloud_sdk.agent_memory.HttpTransport") as MockTransport: + MockTransport.return_value = MagicMock(spec=HttpTransport) + client = create_client( + config=config, + access_strategy=AccessStrategy.SUBSCRIBER, + tenant="acme-corp", + ) + assert client._tenant == "acme-corp" def test_reads_env_when_no_config_provided(self, monkeypatch): """Factory falls back to environment variables when no config given.""" import json - monkeypatch.setenv("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", "http://memory.example.com") - monkeypatch.setenv("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", json.dumps({ - "url": "http://auth.example.com", - "clientid": "client-id", - "clientsecret": "client-secret", - })) + + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", + "http://memory.example.com", + ) + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", + json.dumps( + { + "url": "http://auth.example.com", + "clientid": "client-id", + "clientsecret": "client-secret", + } + ), + ) with patch("sap_cloud_sdk.agent_memory.HttpTransport") as MockTransport: MockTransport.return_value = MagicMock(spec=HttpTransport) - client = create_client() + client = create_client(access_strategy=AccessStrategy.PROVIDER) assert isinstance(client, AgentMemoryClient) +# ── 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() @@ -87,7 +183,10 @@ 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"}) @@ -99,7 +198,10 @@ 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") @@ -112,7 +214,10 @@ 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") @@ -124,7 +229,10 @@ 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") @@ -177,7 +285,6 @@ 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() @@ -323,7 +430,6 @@ 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() @@ -369,14 +475,25 @@ 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, + }, ] } @@ -429,7 +546,6 @@ 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() @@ -443,7 +559,11 @@ 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) @@ -460,8 +580,12 @@ 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") @@ -473,12 +597,18 @@ 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"} @@ -487,8 +617,12 @@ 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") @@ -500,8 +634,12 @@ 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") @@ -525,15 +663,18 @@ 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", }, ], } @@ -549,8 +690,10 @@ 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"] @@ -687,12 +830,13 @@ 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", @@ -737,7 +881,6 @@ 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() @@ -749,19 +892,13 @@ def test_close_delegates_to_transport(self): def test_context_manager_closes_on_exit(self): """Using the client as a context manager closes it on __exit__.""" transport = MagicMock(spec=HttpTransport) - client = AgentMemoryClient(transport) - - with client: - pass - - transport.close.assert_called_once() + client = AgentMemoryClient(transport, access_strategy=AccessStrategy.PROVIDER) # ── Validation ──────────────────────────────────────────────────────────────── class TestMemoryValidation: - def test_add_memory_raises_for_empty_agent_id(self): """add_memory raises AgentMemoryValidationError when agent_id is empty.""" client, _ = _make_client() @@ -818,7 +955,6 @@ 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() @@ -879,7 +1015,6 @@ 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() @@ -930,7 +1065,6 @@ 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() @@ -968,7 +1102,6 @@ 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 d3e895a1..40e9b2ea 100644 --- a/tests/agent_memory/unit/test_config.py +++ b/tests/agent_memory/unit/test_config.py @@ -51,6 +51,14 @@ def test_timeout_default(self): config = AgentMemoryConfig(base_url="http://localhost:8080") assert config.timeout == 30.0 + def test_raises_when_identityzone_empty_string(self): + with pytest.raises(AgentMemoryConfigError, match="identityzone"): + AgentMemoryConfig(base_url="http://localhost", identityzone="") + + def test_identityzone_defaults_to_none(self): + config = AgentMemoryConfig(base_url="http://localhost:8080") + assert config.identityzone is None + def test_valid_config_with_all_fields_does_not_raise(self): AgentMemoryConfig( base_url="https://memory.example.com", @@ -102,6 +110,20 @@ def test_extract_config_raises_on_missing_json_key(self): with pytest.raises(AgentMemoryConfigError, match="Missing required field in uaa JSON"): BindingData(application_url="https://memory.example.com", uaa=uaa).extract_config() + def test_extract_config_maps_identityzone_when_present(self): + uaa = json.dumps({ + "url": "https://my-zone.authentication.eu12.hana.ondemand.com", + "clientid": "c", + "clientsecret": "s", + "identityzone": "my-zone", + }) + config = BindingData(application_url="https://memory.example.com", uaa=uaa).extract_config() + assert config.identityzone == "my-zone" + + def test_extract_config_identityzone_is_none_when_absent(self): + config = BindingData(application_url="https://memory.example.com", uaa=_VALID_UAA).extract_config() + assert config.identityzone is None + def test_extract_config_ignores_extra_uaa_fields(self): uaa = json.dumps({ "apiurl": "https://api.authentication.eu12.hana.ondemand.com", @@ -119,6 +141,7 @@ def test_extract_config_ignores_extra_uaa_fields(self): assert config.token_url == "https://auth.example.com/oauth/token" assert config.client_id == "my-client" assert config.client_secret == "my-secret" + assert config.identityzone == "my-zone" def test_extract_config_raises_on_empty_uaa_object(self): with pytest.raises(AgentMemoryConfigError, match="Missing required field in uaa JSON"): diff --git a/tests/agent_memory/unit/test_http_transport.py b/tests/agent_memory/unit/test_http_transport.py index 282abe37..74008c44 100644 --- a/tests/agent_memory/unit/test_http_transport.py +++ b/tests/agent_memory/unit/test_http_transport.py @@ -13,13 +13,18 @@ from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryHttpError, AgentMemoryNotFoundError -def _config(with_auth: bool = True) -> AgentMemoryConfig: +def _config( + with_auth: bool = True, + identityzone: str | None = None, + token_url: str = "http://auth.example.com/oauth/token", +) -> AgentMemoryConfig: if with_auth: return AgentMemoryConfig( base_url="http://localhost:8080", - token_url="http://localhost:8080/oauth/token", + token_url=token_url, client_id="client-id", client_secret="client-secret", + identityzone=identityzone, ) return AgentMemoryConfig(base_url="http://localhost:8080") @@ -74,7 +79,7 @@ def test_uses_plain_session_when_no_token_url(self): class TestTokenAcquisition: def test_token_is_fetched_and_cached(self): - """fetch_token is called only once across multiple requests.""" + """fetch_token is called only once across multiple requests with the same tenant.""" with patch( "sap_cloud_sdk.agent_memory._http_transport.OAuth2Session" ) as MockOAuth, patch( @@ -110,11 +115,11 @@ def test_expired_token_triggers_refetch(self): mock_oauth.request.return_value = _mock_response(200, {}) transport = HttpTransport(_config()) - transport._token_expires_at = datetime.now() - timedelta(seconds=1) - transport.get("/test") + # Force the cache to have an expired entry + past = datetime.now() - timedelta(seconds=1) + transport._oauth_cache[None] = (mock_oauth, past) transport.get("/test") - # Two fetches: one for each request since we started with an expired timestamp assert mock_oauth.fetch_token.call_count >= 1 def test_token_expiry_uses_buffer(self): @@ -135,11 +140,11 @@ def test_token_expiry_uses_buffer(self): transport = HttpTransport(_config()) transport.get("/test") + _, expires_at = transport._oauth_cache[None] expected_max = datetime.now() + timedelta( seconds=3600 - _TOKEN_EXPIRY_BUFFER_SECONDS + 5 ) - assert transport._token_expires_at is not None - assert transport._token_expires_at < expected_max + assert expires_at < expected_max def test_token_fetch_failure_raises_http_error(self): """Failed token fetch raises AgentMemoryHttpError.""" @@ -157,6 +162,110 @@ def test_token_fetch_failure_raises_http_error(self): transport.get("/test") +# ── Per-tenant token derivation ─────────────────────────────────────────────── + + +class TestTenantTokenDerivation: + + def test_subscriber_token_url_replaces_identityzone(self): + """When tenant_subdomain is provided, identityzone is replaced in the token URL.""" + token_url = "http://provider-zone.auth.example.com/oauth/token" + cfg = _config(with_auth=True, identityzone="provider-zone", token_url=token_url) + + captured_urls = [] + + def fake_fetch_token(**kwargs): + captured_urls.append(kwargs["token_url"]) + return {"access_token": "tok", "expires_in": 3600} + + with patch( + "sap_cloud_sdk.agent_memory._http_transport.OAuth2Session" + ) as MockOAuth, patch( + "sap_cloud_sdk.agent_memory._http_transport.BackendApplicationClient" + ): + mock_oauth = MagicMock() + MockOAuth.return_value = mock_oauth + mock_oauth.fetch_token.side_effect = fake_fetch_token + mock_oauth.request.return_value = _mock_response(200, {}) + + transport = HttpTransport(cfg) + transport.get("/test", tenant_subdomain="subscriber-zone") + + assert len(captured_urls) == 1 + assert "subscriber-zone" in captured_urls[0] + assert "provider-zone" not in captured_urls[0] + + def test_provider_token_url_unchanged_when_no_tenant(self): + """Without tenant_subdomain, the provider token URL is used as-is.""" + token_url = "http://provider-zone.auth.example.com/oauth/token" + cfg = _config(with_auth=True, identityzone="provider-zone", token_url=token_url) + + captured_urls = [] + + def fake_fetch_token(**kwargs): + captured_urls.append(kwargs["token_url"]) + return {"access_token": "tok", "expires_in": 3600} + + with patch( + "sap_cloud_sdk.agent_memory._http_transport.OAuth2Session" + ) as MockOAuth, patch( + "sap_cloud_sdk.agent_memory._http_transport.BackendApplicationClient" + ): + mock_oauth = MagicMock() + MockOAuth.return_value = mock_oauth + mock_oauth.fetch_token.side_effect = fake_fetch_token + mock_oauth.request.return_value = _mock_response(200, {}) + + transport = HttpTransport(cfg) + transport.get("/test") # no tenant_subdomain → None + + assert captured_urls[0] == token_url + + def test_tokens_cached_independently_per_tenant(self): + """Provider and subscriber tokens are cached under separate keys.""" + token_url = "http://prov.auth.example.com/oauth/token" + cfg = _config(with_auth=True, identityzone="prov", token_url=token_url) + + with patch( + "sap_cloud_sdk.agent_memory._http_transport.OAuth2Session" + ) as MockOAuth, patch( + "sap_cloud_sdk.agent_memory._http_transport.BackendApplicationClient" + ): + mock_oauth = MagicMock() + MockOAuth.return_value = mock_oauth + mock_oauth.fetch_token.return_value = {"access_token": "tok", "expires_in": 3600} + mock_oauth.request.return_value = _mock_response(200, {}) + + transport = HttpTransport(cfg) + transport.get("/test") # provider (None) + transport.get("/test", tenant_subdomain="sub") # subscriber + + assert None in transport._oauth_cache + assert "sub" in transport._oauth_cache + assert mock_oauth.fetch_token.call_count == 2 + + def test_subscriber_token_reused_on_second_call(self): + """Subscriber token is cached and not re-fetched on a second call.""" + token_url = "http://prov.auth.example.com/oauth/token" + cfg = _config(with_auth=True, identityzone="prov", token_url=token_url) + + with patch( + "sap_cloud_sdk.agent_memory._http_transport.OAuth2Session" + ) as MockOAuth, patch( + "sap_cloud_sdk.agent_memory._http_transport.BackendApplicationClient" + ): + mock_oauth = MagicMock() + MockOAuth.return_value = mock_oauth + mock_oauth.fetch_token.return_value = {"access_token": "tok", "expires_in": 3600} + mock_oauth.request.return_value = _mock_response(200, {}) + + transport = HttpTransport(cfg) + transport.get("/test", tenant_subdomain="sub") + transport.get("/test", tenant_subdomain="sub") + + assert mock_oauth.fetch_token.call_count == 1 + + # ── HTTP methods ────────────────────────────────────────────────────────────── @@ -242,8 +351,8 @@ def test_server_error_raises_http_error(self): class TestClose: - def test_close_clears_oauth_session(self): - """close() clears the OAuth session.""" + def test_close_clears_all_oauth_sessions(self): + """close() closes all cached OAuth sessions.""" with patch( "sap_cloud_sdk.agent_memory._http_transport.OAuth2Session" ) as MockOAuth, patch( @@ -254,12 +363,15 @@ def test_close_clears_oauth_session(self): mock_oauth.fetch_token.return_value = {"access_token": "tok", "expires_in": 3600} mock_oauth.request.return_value = _mock_response(200, {}) - transport = HttpTransport(_config()) - transport.get("/test") + token_url = "http://prov.auth.example.com/oauth/token" + cfg = _config(with_auth=True, identityzone="prov", token_url=token_url) + transport = HttpTransport(cfg) + transport.get("/test") # provider + transport.get("/test", tenant_subdomain="sub") # subscriber transport.close() - mock_oauth.close.assert_called_once() - assert transport._oauth is None + assert mock_oauth.close.call_count == 2 + assert len(transport._oauth_cache) == 0 def test_close_clears_plain_session(self): """close() clears the plain session in no-auth mode."""