From 37e7f4d2e69e52b49e813529ceb8239181eb34fd Mon Sep 17 00:00:00 2001 From: shiminshen Date: Tue, 2 Jun 2026 15:26:05 +0800 Subject: [PATCH] fix(telephony): resolve transfer context via call-sid index instead of KEYS scan (#387) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transfer-context lookup by original_call_sid ran `redis.keys("transfer:context:*")` and iterated every match — an O(N) blocking scan on call-control hot paths, duplicated across the ARI manager and the Twilio/Telnyx conference strategies. Maintain a `transfer:by_call_sid:{original_call_sid}` -> transfer_id secondary index, written and cleared alongside the context in store/remove, and resolve lookups with a direct GET. Route the Twilio/Telnyx strategies through the manager so the lookup lives in one place (also dropping per-call ad-hoc Redis connections). Closes #328 Co-authored-by: shiminshen <16914659+shiminshen@users.noreply.github.com> --- .../telephony/call_transfer_manager.py | 44 ++++--- .../telephony/providers/telnyx/strategies.py | 25 ++-- .../telephony/providers/twilio/strategies.py | 24 +--- .../telephony/transfer_event_protocol.py | 9 ++ .../telephony/test_call_transfer_manager.py | 111 ++++++++++++++++++ 5 files changed, 160 insertions(+), 53 deletions(-) create mode 100644 api/tests/telephony/test_call_transfer_manager.py diff --git a/api/services/telephony/call_transfer_manager.py b/api/services/telephony/call_transfer_manager.py index a6f0f8c..56c45a2 100644 --- a/api/services/telephony/call_transfer_manager.py +++ b/api/services/telephony/call_transfer_manager.py @@ -47,6 +47,11 @@ class CallTransferManager: redis = await self._get_redis() key = TransferRedisChannels.transfer_context_key(context.transfer_id) await redis.setex(key, ttl, context.to_json()) + if context.original_call_sid: + index_key = TransferRedisChannels.transfer_context_by_call_sid_key( + context.original_call_sid + ) + await redis.setex(index_key, ttl, context.transfer_id) logger.debug(f"Stored transfer context for {context.transfer_id}") except Exception as e: logger.error(f"Failed to store transfer context: {e}") @@ -79,8 +84,15 @@ class CallTransferManager: """ try: redis = await self._get_redis() + context = await self.get_transfer_context(transfer_id) key = TransferRedisChannels.transfer_context_key(transfer_id) - await redis.delete(key) + if context and context.original_call_sid: + index_key = TransferRedisChannels.transfer_context_by_call_sid_key( + context.original_call_sid + ) + await redis.delete(key, index_key) + else: + await redis.delete(key) logger.debug(f"Removed transfer context for {transfer_id}") except Exception as e: logger.error(f"Failed to remove transfer context: {e}") @@ -186,24 +198,24 @@ class CallTransferManager: logger.error(f"Error closing pubsub connection: {e}") async def find_transfer_context_for_call(self, caller_channel_id: str): - """Find the active transfer context for this caller channel.""" - - redis = await self._get_redis() + """Find the active transfer context for this caller channel. + Resolves via the original_call_sid -> transfer_id secondary index + (see store_transfer_context) instead of scanning the keyspace with + ``KEYS transfer:context:*``. + """ try: - # Search Redis for transfer contexts where original_call_sid matches this caller - transfer_keys = await redis.keys("transfer:context:*") - - for key in transfer_keys: - try: - context_data = await redis.get(key) - if context_data: - context = TransferContext.from_json(context_data) - if context.original_call_sid == caller_channel_id: - return context - except Exception: - continue + redis = await self._get_redis() + index_key = TransferRedisChannels.transfer_context_by_call_sid_key( + caller_channel_id + ) + transfer_id = await redis.get(index_key) + if not transfer_id: + return None + context = await self.get_transfer_context(transfer_id) + if context and context.original_call_sid == caller_channel_id: + return context return None except Exception as e: diff --git a/api/services/telephony/providers/telnyx/strategies.py b/api/services/telephony/providers/telnyx/strategies.py index 16464f3..0f01c64 100644 --- a/api/services/telephony/providers/telnyx/strategies.py +++ b/api/services/telephony/providers/telnyx/strategies.py @@ -116,25 +116,14 @@ class TelnyxConferenceStrategy(TransferStrategy): async def _find_transfer_context_for_call(self, caller_call_control_id: str): """Find the active transfer context whose original_call_sid matches.""" try: - import redis.asyncio as aioredis + from api.services.telephony.call_transfer_manager import ( + get_call_transfer_manager, + ) - from api.constants import REDIS_URL - from api.services.telephony.transfer_event_protocol import TransferContext - - redis = aioredis.from_url(REDIS_URL, decode_responses=True) - transfer_keys = await redis.keys("transfer:context:*") - - for key in transfer_keys: - try: - context_data = await redis.get(key) - if context_data: - context = TransferContext.from_json(context_data) - if context.original_call_sid == caller_call_control_id: - return context - except Exception: - continue - - return None + manager = await get_call_transfer_manager() + return await manager.find_transfer_context_for_call( + caller_call_control_id + ) except Exception as e: logger.error(f"[Telnyx Transfer] Error finding transfer context: {e}") diff --git a/api/services/telephony/providers/twilio/strategies.py b/api/services/telephony/providers/twilio/strategies.py index e80e1a6..bca1482 100644 --- a/api/services/telephony/providers/twilio/strategies.py +++ b/api/services/telephony/providers/twilio/strategies.py @@ -106,26 +106,12 @@ class TwilioConferenceStrategy(TransferStrategy): async def _find_transfer_context_for_call(self, call_sid: str): """Find the active transfer context for this call.""" try: - import redis.asyncio as aioredis + from api.services.telephony.call_transfer_manager import ( + get_call_transfer_manager, + ) - from api.constants import REDIS_URL - from api.services.telephony.transfer_event_protocol import TransferContext - - # Search Redis for transfer contexts where original_call_sid matches - redis = aioredis.from_url(REDIS_URL, decode_responses=True) - transfer_keys = await redis.keys("transfer:context:*") - - for key in transfer_keys: - try: - context_data = await redis.get(key) - if context_data: - context = TransferContext.from_json(context_data) - if context.original_call_sid == call_sid: - return context - except Exception: - continue - - return None + call_transfer_manager = await get_call_transfer_manager() + return await call_transfer_manager.find_transfer_context_for_call(call_sid) except Exception as e: logger.error(f"[Twilio Transfer] Error finding transfer context: {e}") diff --git a/api/services/telephony/transfer_event_protocol.py b/api/services/telephony/transfer_event_protocol.py index e47df08..19070ff 100644 --- a/api/services/telephony/transfer_event_protocol.py +++ b/api/services/telephony/transfer_event_protocol.py @@ -99,3 +99,12 @@ class TransferRedisChannels: def transfer_context_key(transfer_id: str) -> str: """Redis key for transfer context storage.""" return f"transfer:context:{transfer_id}" + + @staticmethod + def transfer_context_by_call_sid_key(original_call_sid: str) -> str: + """Redis key for the original_call_sid -> transfer_id secondary index. + + Lets a caller's transfer context be resolved with a direct lookup + instead of an O(N) ``KEYS transfer:context:*`` keyspace scan. + """ + return f"transfer:by_call_sid:{original_call_sid}" diff --git a/api/tests/telephony/test_call_transfer_manager.py b/api/tests/telephony/test_call_transfer_manager.py new file mode 100644 index 0000000..dce422b --- /dev/null +++ b/api/tests/telephony/test_call_transfer_manager.py @@ -0,0 +1,111 @@ +"""Tests for CallTransferManager Redis-backed transfer-context lookup. + +These tests verify (regression for issue #328): +1. Lookup by original_call_sid resolves via a secondary index, never an + O(N) `KEYS transfer:context:*` keyspace scan. +2. A lookup for an unknown call sid returns None without scanning. +3. Removing a transfer context also clears its call-sid index entry. +""" + +from typing import Dict, List + +import pytest + + +class _FakeRedis: + """Minimal in-memory async Redis double. + + Counts calls to ``keys()`` so tests can assert the lookup path no longer + performs an O(N) keyspace scan (the regression behind issue #328). + """ + + def __init__(self) -> None: + self._store: Dict[str, str] = {} + self.keys_call_count = 0 + + async def setex(self, key: str, ttl: int, value: str) -> None: + self._store[key] = value + + async def get(self, key: str): + return self._store.get(key) + + async def delete(self, *keys: str) -> None: + for key in keys: + self._store.pop(key, None) + + async def keys(self, pattern: str) -> List[str]: + self.keys_call_count += 1 + if pattern.endswith("*"): + prefix = pattern[:-1] + return [k for k in self._store if k.startswith(prefix)] + return [k for k in self._store if k == pattern] + + +def _build_context(transfer_id: str, original_call_sid: str): + from api.services.telephony.transfer_event_protocol import TransferContext + + return TransferContext( + transfer_id=transfer_id, + call_sid="dest-call-sid", + target_number="+15551230000", + tool_uuid="tool-uuid", + original_call_sid=original_call_sid, + conference_name="conference-name", + initiated_at=0.0, + ) + + +class TestFindTransferContextByCallSid: + """Lookup must use the call-sid index, not a KEYS scan (issue #328).""" + + @pytest.mark.asyncio + async def test_lookup_uses_index_and_not_keys_scan(self): + from api.services.telephony.call_transfer_manager import CallTransferManager + + fake = _FakeRedis() + manager = CallTransferManager(redis_client=fake) + + await manager.store_transfer_context( + _build_context("tx-1", "caller-abc") + ) + + found = await manager.find_transfer_context_for_call("caller-abc") + + assert found is not None + assert found.transfer_id == "tx-1" + # Regression (issue #328): the lookup must resolve via the secondary + # index, never an O(N) `KEYS transfer:context:*` keyspace scan. + assert fake.keys_call_count == 0 + + @pytest.mark.asyncio + async def test_lookup_returns_none_for_unknown_call_sid(self): + from api.services.telephony.call_transfer_manager import CallTransferManager + + fake = _FakeRedis() + manager = CallTransferManager(redis_client=fake) + + await manager.store_transfer_context( + _build_context("tx-1", "caller-abc") + ) + + found = await manager.find_transfer_context_for_call("not-a-caller") + + assert found is None + assert fake.keys_call_count == 0 + + @pytest.mark.asyncio + async def test_remove_clears_call_sid_index(self): + from api.services.telephony.call_transfer_manager import CallTransferManager + + fake = _FakeRedis() + manager = CallTransferManager(redis_client=fake) + + await manager.store_transfer_context( + _build_context("tx-1", "caller-abc") + ) + await manager.remove_transfer_context("tx-1") + + found = await manager.find_transfer_context_for_call("caller-abc") + + assert found is None + assert fake.keys_call_count == 0