mirror of
https://github.com/dograh-hq/dograh.git
synced 2026-07-10 11:12:13 +02:00
fix: increase concurrency limit an handle it across all call paths (#508)
* fix: increase concurrency limit an handle it across all call paths * fix: fix review comments and test * fix: address concurrency review findings (campaign-scoped counter, cleanup hardening, webrtc run validation) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: emit usage_concurrent_call_limit_reached PostHog event Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: align usage event with MPS org-event convention (per-member fan-out) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: cap max_call_duration at 20 min via typed workflow_configurations request Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
f3bcf24370
commit
041c31a613
40 changed files with 2369 additions and 467 deletions
|
|
@ -244,3 +244,34 @@ def test_parse_cloudonix_cdr_preserves_zero_billsec():
|
|||
)
|
||||
|
||||
assert req["duration"] == "0"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_stream_handshake_timeout_closes_socket(monkeypatch):
|
||||
"""An idle agent-stream socket holds an org concurrency slot, so the
|
||||
handshake read must be bounded rather than waiting forever."""
|
||||
import asyncio
|
||||
|
||||
from api.services.telephony.providers.cloudonix import provider as provider_module
|
||||
|
||||
monkeypatch.setattr(provider_module, "AGENT_STREAM_HANDSHAKE_TIMEOUT_S", 0.05)
|
||||
provider = CloudonixProvider({})
|
||||
|
||||
async def never_returns():
|
||||
await asyncio.Event().wait()
|
||||
|
||||
websocket = SimpleNamespace(receive_text=never_returns, close=AsyncMock())
|
||||
|
||||
await asyncio.wait_for(
|
||||
provider.handle_external_websocket(
|
||||
websocket,
|
||||
organization_id=1,
|
||||
workflow_id=2,
|
||||
user_id=3,
|
||||
workflow_run_id=4,
|
||||
params={},
|
||||
),
|
||||
timeout=5,
|
||||
)
|
||||
|
||||
websocket.close.assert_awaited_once_with(code=4408, reason="Handshake timeout")
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import json
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
|
@ -7,6 +8,13 @@ from api.services.telephony import ari_manager
|
|||
from api.services.telephony.ari_manager import ARIConnection
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fake_call_concurrency(monkeypatch):
|
||||
fake = SimpleNamespace(unregister_active_call=AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(ari_manager, "call_concurrency", fake)
|
||||
return fake
|
||||
|
||||
|
||||
class _FakeDbClient:
|
||||
def __init__(self, gathered_context):
|
||||
self.workflow_run = SimpleNamespace(gathered_context=gathered_context)
|
||||
|
|
@ -77,6 +85,7 @@ def _completed_transfer_context():
|
|||
@pytest.mark.asyncio
|
||||
async def test_completed_transfer_tears_down_destination_when_caller_leaves(
|
||||
monkeypatch,
|
||||
fake_call_concurrency,
|
||||
):
|
||||
fake_db = _FakeDbClient(_completed_transfer_context())
|
||||
monkeypatch.setattr(ari_manager, "db_client", fake_db)
|
||||
|
|
@ -90,11 +99,13 @@ async def test_completed_transfer_tears_down_destination_when_caller_leaves(
|
|||
assert "dest-chan" in conn.deleted_channel_runs
|
||||
assert conn.deleted_transfer_channel_mappings == ["dest-chan"]
|
||||
assert fake_db.workflow_run.gathered_context["transfer_state"] == "terminated"
|
||||
fake_call_concurrency.unregister_active_call.assert_awaited_with(123)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_completed_transfer_tears_down_caller_when_destination_leaves(
|
||||
monkeypatch,
|
||||
fake_call_concurrency,
|
||||
):
|
||||
fake_db = _FakeDbClient(_completed_transfer_context())
|
||||
monkeypatch.setattr(ari_manager, "db_client", fake_db)
|
||||
|
|
@ -108,6 +119,31 @@ async def test_completed_transfer_tears_down_caller_when_destination_leaves(
|
|||
assert "dest-chan" in conn.deleted_channel_runs
|
||||
assert conn.deleted_transfer_channel_mappings == ["dest-chan"]
|
||||
assert fake_db.workflow_run.gathered_context["transfer_state"] == "terminated"
|
||||
fake_call_concurrency.unregister_active_call.assert_awaited_with(123)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stasis_end_releases_concurrency_slot_on_normal_teardown(
|
||||
monkeypatch,
|
||||
fake_call_concurrency,
|
||||
):
|
||||
"""StasisEnd must release the org slot even when the pipeline never ran
|
||||
(e.g. the caller hung up before external media connected)."""
|
||||
fake_db = _FakeDbClient(
|
||||
{
|
||||
"call_id": "caller-chan",
|
||||
"ext_channel_id": "ext-chan",
|
||||
"bridge_id": "bridge-1",
|
||||
}
|
||||
)
|
||||
monkeypatch.setattr(ari_manager, "db_client", fake_db)
|
||||
conn = _RecordingARIConnection()
|
||||
|
||||
await conn._handle_stasis_end("caller-chan", "123")
|
||||
|
||||
fake_call_concurrency.unregister_active_call.assert_awaited_once_with(123)
|
||||
assert conn.deleted_bridges == ["bridge-1"]
|
||||
assert conn.deleted_channels == ["ext-chan"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -29,6 +29,9 @@ async def test_initialized_no_answer_enqueues_workflow_completion():
|
|||
|
||||
with (
|
||||
patch("api.services.telephony.status_processor.db_client") as mock_db,
|
||||
patch(
|
||||
"api.services.telephony.status_processor.campaign_call_dispatcher"
|
||||
) as mock_dispatcher,
|
||||
patch(
|
||||
"api.services.telephony.status_processor.enqueue_job",
|
||||
new_callable=AsyncMock,
|
||||
|
|
@ -36,6 +39,7 @@ async def test_initialized_no_answer_enqueues_workflow_completion():
|
|||
):
|
||||
mock_db.get_workflow_run_by_id = AsyncMock(return_value=workflow_run)
|
||||
mock_db.update_workflow_run = AsyncMock()
|
||||
mock_dispatcher.release_call_slot = AsyncMock(return_value=True)
|
||||
|
||||
await _process_status_update(123, status)
|
||||
|
||||
|
|
@ -58,6 +62,7 @@ async def test_initialized_no_answer_enqueues_workflow_completion():
|
|||
mock_enqueue.assert_awaited_once_with(
|
||||
FunctionNames.RUN_INTEGRATIONS_POST_WORKFLOW_RUN, 123
|
||||
)
|
||||
mock_dispatcher.release_call_slot.assert_awaited_once_with(123)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -79,6 +84,9 @@ async def test_running_terminal_status_does_not_enqueue_workflow_completion():
|
|||
|
||||
with (
|
||||
patch("api.services.telephony.status_processor.db_client") as mock_db,
|
||||
patch(
|
||||
"api.services.telephony.status_processor.campaign_call_dispatcher"
|
||||
) as mock_dispatcher,
|
||||
patch(
|
||||
"api.services.telephony.status_processor.enqueue_job",
|
||||
new_callable=AsyncMock,
|
||||
|
|
@ -86,6 +94,7 @@ async def test_running_terminal_status_does_not_enqueue_workflow_completion():
|
|||
):
|
||||
mock_db.get_workflow_run_by_id = AsyncMock(return_value=workflow_run)
|
||||
mock_db.update_workflow_run = AsyncMock()
|
||||
mock_dispatcher.release_call_slot = AsyncMock(return_value=True)
|
||||
|
||||
await _process_status_update(456, status)
|
||||
|
||||
|
|
@ -96,3 +105,4 @@ async def test_running_terminal_status_does_not_enqueue_workflow_completion():
|
|||
"telephony_failed",
|
||||
]
|
||||
mock_enqueue.assert_not_awaited()
|
||||
mock_dispatcher.release_call_slot.assert_awaited_once_with(456)
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ idempotent, and the count only reaches zero when every registered run is gone.
|
|||
"""
|
||||
|
||||
import asyncio
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
|
|
@ -91,6 +92,12 @@ async def test_run_pipeline_counts_call_during_setup(monkeypatch):
|
|||
"get_workflow_run",
|
||||
fake_get_workflow_run,
|
||||
)
|
||||
unregister_concurrency = AsyncMock()
|
||||
monkeypatch.setattr(
|
||||
run_pipeline_module.call_concurrency,
|
||||
"unregister_active_call",
|
||||
unregister_concurrency,
|
||||
)
|
||||
|
||||
task = asyncio.create_task(
|
||||
run_pipeline_module._run_pipeline(
|
||||
|
|
@ -108,6 +115,7 @@ async def test_run_pipeline_counts_call_during_setup(monkeypatch):
|
|||
with pytest.raises(RuntimeError, match="setup failed"):
|
||||
await asyncio.wait_for(task, timeout=1.0)
|
||||
assert active_calls.active_call_count() == 0
|
||||
unregister_concurrency.assert_awaited_once_with(42)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -123,6 +131,12 @@ async def test_webrtc_entrypoint_counts_call_during_setup(monkeypatch):
|
|||
monkeypatch.setattr(
|
||||
run_pipeline_module.db_client, "get_workflow", fake_get_workflow
|
||||
)
|
||||
unregister_concurrency = AsyncMock()
|
||||
monkeypatch.setattr(
|
||||
run_pipeline_module.call_concurrency,
|
||||
"unregister_active_call",
|
||||
unregister_concurrency,
|
||||
)
|
||||
|
||||
task = asyncio.create_task(
|
||||
run_pipeline_module.run_pipeline_smallwebrtc(
|
||||
|
|
@ -140,6 +154,7 @@ async def test_webrtc_entrypoint_counts_call_during_setup(monkeypatch):
|
|||
with pytest.raises(RuntimeError, match="setup failed"):
|
||||
await asyncio.wait_for(task, timeout=1.0)
|
||||
assert active_calls.active_call_count() == 0
|
||||
unregister_concurrency.assert_awaited_once_with(43)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -155,6 +170,12 @@ async def test_telephony_entrypoint_counts_call_during_setup(monkeypatch):
|
|||
monkeypatch.setattr(
|
||||
run_pipeline_module.db_client, "get_workflow", fake_get_workflow
|
||||
)
|
||||
unregister_concurrency = AsyncMock()
|
||||
monkeypatch.setattr(
|
||||
run_pipeline_module.call_concurrency,
|
||||
"unregister_active_call",
|
||||
unregister_concurrency,
|
||||
)
|
||||
|
||||
task = asyncio.create_task(
|
||||
run_pipeline_module.run_pipeline_telephony(
|
||||
|
|
@ -175,6 +196,7 @@ async def test_telephony_entrypoint_counts_call_during_setup(monkeypatch):
|
|||
with pytest.raises(RuntimeError, match="setup failed"):
|
||||
await asyncio.wait_for(task, timeout=1.0)
|
||||
assert active_calls.active_call_count() == 0
|
||||
unregister_concurrency.assert_awaited_once_with(44)
|
||||
|
||||
|
||||
def test_active_calls_route_requires_configured_secret(monkeypatch):
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@ from unittest.mock import AsyncMock, patch
|
|||
|
||||
import pytest
|
||||
|
||||
from api.services.call_concurrency import CallConcurrencyLimitError
|
||||
|
||||
|
||||
class _FakeWebSocket:
|
||||
def __init__(self, query_params: dict[str, str] | None = None):
|
||||
|
|
@ -34,6 +36,7 @@ async def test_agent_stream_uses_provider_path_param_not_query_param():
|
|||
with (
|
||||
patch("api.routes.agent_stream.telephony_registry") as registry,
|
||||
patch("api.routes.agent_stream.db_client") as db_client,
|
||||
patch("api.routes.agent_stream.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.agent_stream.authorize_workflow_run_start",
|
||||
new=AsyncMock(
|
||||
|
|
@ -41,6 +44,13 @@ async def test_agent_stream_uses_provider_path_param_not_query_param():
|
|||
),
|
||||
),
|
||||
):
|
||||
slot = object()
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(return_value=slot)
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
mock_concurrency.unregister_active_call = AsyncMock()
|
||||
|
||||
registry.get_optional.return_value = spec
|
||||
db_client.get_workflow_by_uuid_unscoped = AsyncMock(return_value=workflow)
|
||||
db_client.create_workflow_run = AsyncMock(return_value=workflow_run)
|
||||
|
|
@ -50,9 +60,17 @@ async def test_agent_stream_uses_provider_path_param_not_query_param():
|
|||
|
||||
registry.get_optional.assert_called_once_with("cloudonix")
|
||||
db_client.create_workflow_run.assert_awaited_once()
|
||||
mock_concurrency.acquire_org_slot.assert_awaited_once_with(
|
||||
workflow.organization_id,
|
||||
source="agent_stream:cloudonix",
|
||||
timeout=0,
|
||||
)
|
||||
mock_concurrency.bind_workflow_run.assert_awaited_once_with(slot, workflow_run.id)
|
||||
mock_concurrency.unregister_active_call.assert_awaited_once_with(workflow_run.id)
|
||||
create_args = db_client.create_workflow_run.await_args.args
|
||||
create_kwargs = db_client.create_workflow_run.await_args.kwargs
|
||||
assert create_args[2] == "cloudonix"
|
||||
assert create_kwargs["organization_id"] == workflow.organization_id
|
||||
assert create_kwargs["initial_context"] == {
|
||||
"existing": "context",
|
||||
"provider": "cloudonix",
|
||||
|
|
@ -62,3 +80,42 @@ async def test_agent_stream_uses_provider_path_param_not_query_param():
|
|||
_, provider_kwargs = provider.handle_external_websocket.await_args
|
||||
assert provider_kwargs["params"] == {"custom": "value"}
|
||||
websocket.close.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_stream_rejects_when_concurrency_limit_reached():
|
||||
from api.routes.agent_stream import agent_stream_websocket
|
||||
|
||||
websocket = _FakeWebSocket()
|
||||
workflow = SimpleNamespace(
|
||||
id=11,
|
||||
user_id=22,
|
||||
organization_id=33,
|
||||
template_context_variables={},
|
||||
)
|
||||
spec = SimpleNamespace(provider_cls=lambda _config: object())
|
||||
|
||||
with (
|
||||
patch("api.routes.agent_stream.telephony_registry") as registry,
|
||||
patch("api.routes.agent_stream.db_client") as db_client,
|
||||
patch("api.routes.agent_stream.call_concurrency") as mock_concurrency,
|
||||
):
|
||||
registry.get_optional.return_value = spec
|
||||
db_client.get_workflow_by_uuid_unscoped = AsyncMock(return_value=workflow)
|
||||
db_client.create_workflow_run = AsyncMock()
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(
|
||||
side_effect=CallConcurrencyLimitError(
|
||||
organization_id=workflow.organization_id,
|
||||
source="agent_stream:cloudonix",
|
||||
wait_time=0,
|
||||
max_concurrent=1,
|
||||
)
|
||||
)
|
||||
|
||||
await agent_stream_websocket(websocket, "cloudonix", "agent-uuid")
|
||||
|
||||
websocket.close.assert_awaited_once_with(
|
||||
code=1008,
|
||||
reason="Concurrent call limit reached",
|
||||
)
|
||||
db_client.create_workflow_run.assert_not_awaited()
|
||||
|
|
|
|||
333
api/tests/test_call_concurrency.py
Normal file
333
api/tests/test_call_concurrency.py
Normal file
|
|
@ -0,0 +1,333 @@
|
|||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from api.services.call_concurrency import (
|
||||
CallConcurrencyLimitError,
|
||||
CallConcurrencyService,
|
||||
)
|
||||
from api.services.campaign.rate_limiter import ConcurrentSlotAcquisition
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_acquire_org_slot_logs_post_acquire_count_and_limit():
|
||||
service = CallConcurrencyService()
|
||||
|
||||
with (
|
||||
patch("api.services.call_concurrency.db_client") as mock_db,
|
||||
patch("api.services.call_concurrency.rate_limiter") as mock_rate_limiter,
|
||||
patch("api.services.call_concurrency.logger") as mock_logger,
|
||||
):
|
||||
mock_db.get_configuration = AsyncMock(return_value=None)
|
||||
mock_rate_limiter.try_acquire_concurrent_slot_details = AsyncMock(
|
||||
return_value=ConcurrentSlotAcquisition(
|
||||
slot_id="slot-123",
|
||||
active_count=7,
|
||||
)
|
||||
)
|
||||
|
||||
slot = await service.acquire_org_slot(199, source="test_source")
|
||||
|
||||
assert slot.organization_id == 199
|
||||
assert slot.slot_id == "slot-123"
|
||||
assert slot.max_concurrent == 10
|
||||
assert slot.source == "test_source"
|
||||
mock_rate_limiter.try_acquire_concurrent_slot_details.assert_awaited_once_with(
|
||||
199, 10, scope_key=None, scope_max_concurrent=None
|
||||
)
|
||||
mock_logger.info.assert_called_once()
|
||||
log_message = mock_logger.info.call_args.args[0]
|
||||
assert "org 199" in log_message
|
||||
assert "source=test_source" in log_message
|
||||
assert "active_calls=7/10" in log_message
|
||||
assert "slot_id=slot-123" in log_message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_acquire_org_slot_logs_warning_when_limit_reached():
|
||||
service = CallConcurrencyService()
|
||||
|
||||
with (
|
||||
patch("api.services.call_concurrency.db_client") as mock_db,
|
||||
patch("api.services.call_concurrency.rate_limiter") as mock_rate_limiter,
|
||||
patch("api.services.call_concurrency.logger") as mock_logger,
|
||||
):
|
||||
mock_db.get_configuration = AsyncMock(return_value=None)
|
||||
mock_rate_limiter.try_acquire_concurrent_slot_details = AsyncMock(
|
||||
return_value=None
|
||||
)
|
||||
mock_rate_limiter.get_concurrent_count = AsyncMock(return_value=12)
|
||||
|
||||
with pytest.raises(CallConcurrencyLimitError):
|
||||
await service.acquire_org_slot(199, source="test_source", timeout=0)
|
||||
|
||||
mock_rate_limiter.get_concurrent_count.assert_awaited_once_with(199)
|
||||
mock_logger.warning.assert_called_once()
|
||||
log_message = mock_logger.warning.call_args.args[0]
|
||||
assert "Concurrent call limit reached for org 199" in log_message
|
||||
assert "source=test_source" in log_message
|
||||
assert "active_calls=12/10" in log_message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_acquire_org_slot_fires_usage_event_per_org_member_when_limit_reached():
|
||||
"""Mirrors the MPS org-event convention: one event per org member with the
|
||||
member's provider_id as distinct_id, event_source property, no $groups."""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from api.enums import PostHogEvent
|
||||
|
||||
service = CallConcurrencyService()
|
||||
members = [
|
||||
SimpleNamespace(provider_id="user-a"),
|
||||
SimpleNamespace(provider_id="user-b"),
|
||||
]
|
||||
|
||||
with (
|
||||
patch("api.services.call_concurrency.db_client") as mock_db,
|
||||
patch("api.services.call_concurrency.rate_limiter") as mock_rate_limiter,
|
||||
patch("api.services.call_concurrency.capture_event") as mock_capture,
|
||||
):
|
||||
mock_db.get_configuration = AsyncMock(return_value=None)
|
||||
mock_db.get_organization_users = AsyncMock(return_value=members)
|
||||
mock_rate_limiter.try_acquire_concurrent_slot_details = AsyncMock(
|
||||
return_value=None
|
||||
)
|
||||
mock_rate_limiter.get_concurrent_count = AsyncMock(return_value=10)
|
||||
|
||||
with pytest.raises(CallConcurrencyLimitError):
|
||||
await service.acquire_org_slot(199, source="webrtc", timeout=0)
|
||||
|
||||
mock_db.get_organization_users.assert_awaited_once_with(199)
|
||||
assert mock_capture.call_count == 2
|
||||
distinct_ids = [c.kwargs["distinct_id"] for c in mock_capture.call_args_list]
|
||||
assert distinct_ids == ["user-a", "user-b"]
|
||||
for call in mock_capture.call_args_list:
|
||||
kwargs = call.kwargs
|
||||
assert kwargs["event"] == PostHogEvent.USAGE_CONCURRENT_CALL_LIMIT_REACHED
|
||||
assert "groups" not in kwargs
|
||||
assert kwargs["properties"]["event_source"] == "dograh"
|
||||
assert kwargs["properties"]["organization_id"] == 199
|
||||
assert kwargs["properties"]["source"] == "webrtc"
|
||||
assert kwargs["properties"]["active_calls"] == 10
|
||||
assert kwargs["properties"]["max_concurrent"] == 10
|
||||
assert "scope_key" not in kwargs["properties"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_acquire_org_slot_passes_scope_to_rate_limiter():
|
||||
service = CallConcurrencyService()
|
||||
|
||||
with (
|
||||
patch("api.services.call_concurrency.db_client") as mock_db,
|
||||
patch("api.services.call_concurrency.rate_limiter") as mock_rate_limiter,
|
||||
):
|
||||
mock_db.get_configuration = AsyncMock(return_value=None)
|
||||
mock_rate_limiter.try_acquire_concurrent_slot_details = AsyncMock(
|
||||
return_value=ConcurrentSlotAcquisition(slot_id="slot-123", active_count=1)
|
||||
)
|
||||
mock_rate_limiter.store_workflow_slot_mapping_if_absent = AsyncMock(
|
||||
return_value=True
|
||||
)
|
||||
|
||||
slot = await service.acquire_org_slot(
|
||||
199,
|
||||
source="campaign:42",
|
||||
scope_key="campaign:42",
|
||||
scope_max_concurrent=3,
|
||||
)
|
||||
await service.bind_workflow_run(slot, 501)
|
||||
|
||||
assert slot.scope_key == "campaign:42"
|
||||
mock_rate_limiter.try_acquire_concurrent_slot_details.assert_awaited_once_with(
|
||||
199, 10, scope_key="campaign:42", scope_max_concurrent=3
|
||||
)
|
||||
mock_rate_limiter.store_workflow_slot_mapping_if_absent.assert_awaited_once_with(
|
||||
501, 199, "slot-123", scope_key="campaign:42"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_release_workflow_run_slot_keeps_mapping_on_redis_error():
|
||||
service = CallConcurrencyService()
|
||||
|
||||
with patch("api.services.call_concurrency.rate_limiter") as mock_rate_limiter:
|
||||
mock_rate_limiter.get_workflow_slot_mapping = AsyncMock(
|
||||
return_value=(11, "slot-1", None)
|
||||
)
|
||||
# None = Redis error during release (vs False = slot already gone)
|
||||
mock_rate_limiter.release_concurrent_slot = AsyncMock(return_value=None)
|
||||
mock_rate_limiter.delete_workflow_slot_mapping = AsyncMock()
|
||||
|
||||
released = await service.release_workflow_run_slot(501)
|
||||
|
||||
assert released is False
|
||||
mock_rate_limiter.release_concurrent_slot.assert_awaited_once_with(
|
||||
11, "slot-1", scope_key=None
|
||||
)
|
||||
mock_rate_limiter.delete_workflow_slot_mapping.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_release_workflow_run_slot_deletes_mapping_when_slot_already_gone():
|
||||
service = CallConcurrencyService()
|
||||
|
||||
with patch("api.services.call_concurrency.rate_limiter") as mock_rate_limiter:
|
||||
mock_rate_limiter.get_workflow_slot_mapping = AsyncMock(
|
||||
return_value=(11, "slot-1", "campaign:42")
|
||||
)
|
||||
mock_rate_limiter.release_concurrent_slot = AsyncMock(return_value=False)
|
||||
mock_rate_limiter.delete_workflow_slot_mapping = AsyncMock(return_value=True)
|
||||
|
||||
released = await service.release_workflow_run_slot(501)
|
||||
|
||||
assert released is False
|
||||
mock_rate_limiter.release_concurrent_slot.assert_awaited_once_with(
|
||||
11, "slot-1", scope_key="campaign:42"
|
||||
)
|
||||
mock_rate_limiter.delete_workflow_slot_mapping.assert_awaited_once_with(501)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unregister_active_call_never_raises():
|
||||
service = CallConcurrencyService()
|
||||
|
||||
with patch("api.services.call_concurrency.rate_limiter") as mock_rate_limiter:
|
||||
mock_rate_limiter.get_workflow_slot_mapping = AsyncMock(
|
||||
side_effect=RuntimeError("redis down")
|
||||
)
|
||||
|
||||
released = await service.unregister_active_call(501)
|
||||
|
||||
assert released is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Redis integration tests for scoped (campaign-level) slot acquisition
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
import os # noqa: E402
|
||||
import uuid # noqa: E402
|
||||
|
||||
from api.services.campaign.rate_limiter import RateLimiter # noqa: E402
|
||||
|
||||
requires_redis = pytest.mark.skipif(
|
||||
"REDIS_URL" not in os.environ,
|
||||
reason="Requires Redis (set REDIS_URL via .env.test)",
|
||||
)
|
||||
|
||||
|
||||
def _unique_org_id() -> int:
|
||||
return uuid.uuid4().int % 10_000_000
|
||||
|
||||
|
||||
@requires_redis
|
||||
@pytest.mark.asyncio
|
||||
async def test_scoped_acquisition_enforces_scope_limit_independently_of_org():
|
||||
"""A campaign scope caps its own calls without measuring — or being
|
||||
starved by — other calls in the same org counter."""
|
||||
rl = RateLimiter()
|
||||
org_id = _unique_org_id()
|
||||
scope = f"campaign:{org_id}"
|
||||
org_key = f"concurrent_calls:{org_id}"
|
||||
scope_key_full = f"concurrent_calls:{scope}"
|
||||
redis_client = await rl._get_redis()
|
||||
|
||||
try:
|
||||
# Unscoped (e.g. WebRTC) calls fill part of the org counter.
|
||||
for _ in range(3):
|
||||
assert await rl.try_acquire_concurrent_slot_details(org_id, 10)
|
||||
|
||||
# Scope limit 2: two scoped acquisitions succeed...
|
||||
first = await rl.try_acquire_concurrent_slot_details(
|
||||
org_id, 10, scope_key=scope, scope_max_concurrent=2
|
||||
)
|
||||
second = await rl.try_acquire_concurrent_slot_details(
|
||||
org_id, 10, scope_key=scope, scope_max_concurrent=2
|
||||
)
|
||||
assert first and second
|
||||
|
||||
# ...the third is rejected by the scope even though the org has room.
|
||||
third = await rl.try_acquire_concurrent_slot_details(
|
||||
org_id, 10, scope_key=scope, scope_max_concurrent=2
|
||||
)
|
||||
assert third is None
|
||||
|
||||
# Unscoped calls are unaffected by the scope being full.
|
||||
assert await rl.try_acquire_concurrent_slot_details(org_id, 10)
|
||||
|
||||
# Releasing with the scope key frees both counters.
|
||||
released = await rl.release_concurrent_slot(
|
||||
org_id, first.slot_id, scope_key=scope
|
||||
)
|
||||
assert released is True
|
||||
assert await redis_client.zscore(org_key, first.slot_id) is None
|
||||
assert await redis_client.zscore(scope_key_full, first.slot_id) is None
|
||||
|
||||
# And the scope accepts a new call again.
|
||||
assert await rl.try_acquire_concurrent_slot_details(
|
||||
org_id, 10, scope_key=scope, scope_max_concurrent=2
|
||||
)
|
||||
finally:
|
||||
await redis_client.delete(org_key, scope_key_full)
|
||||
await rl.close()
|
||||
|
||||
|
||||
@requires_redis
|
||||
@pytest.mark.asyncio
|
||||
async def test_org_limit_still_binds_scoped_acquisition():
|
||||
rl = RateLimiter()
|
||||
org_id = _unique_org_id()
|
||||
scope = f"campaign:{org_id}"
|
||||
org_key = f"concurrent_calls:{org_id}"
|
||||
scope_key_full = f"concurrent_calls:{scope}"
|
||||
redis_client = await rl._get_redis()
|
||||
|
||||
try:
|
||||
assert await rl.try_acquire_concurrent_slot_details(org_id, 1)
|
||||
|
||||
# Org counter is full, so the scoped acquire fails and must not
|
||||
# leave a phantom entry in the scope counter.
|
||||
rejected = await rl.try_acquire_concurrent_slot_details(
|
||||
org_id, 1, scope_key=scope, scope_max_concurrent=5
|
||||
)
|
||||
assert rejected is None
|
||||
assert await redis_client.zcard(scope_key_full) == 0
|
||||
finally:
|
||||
await redis_client.delete(org_key, scope_key_full)
|
||||
await rl.close()
|
||||
|
||||
|
||||
@requires_redis
|
||||
@pytest.mark.asyncio
|
||||
async def test_workflow_slot_mapping_round_trips_scope_key():
|
||||
rl = RateLimiter()
|
||||
run_id = _unique_org_id()
|
||||
mapping_key = f"workflow_slot_mapping:{run_id}"
|
||||
redis_client = await rl._get_redis()
|
||||
|
||||
try:
|
||||
stored = await rl.store_workflow_slot_mapping_if_absent(
|
||||
run_id, 11, "slot-1", scope_key="campaign:42"
|
||||
)
|
||||
assert stored is True
|
||||
assert await rl.get_workflow_slot_mapping(run_id) == (
|
||||
11,
|
||||
"slot-1",
|
||||
"campaign:42",
|
||||
)
|
||||
|
||||
# Unscoped mappings surface scope_key=None.
|
||||
run_id_2 = _unique_org_id()
|
||||
try:
|
||||
await rl.store_workflow_slot_mapping_if_absent(run_id_2, 11, "slot-2")
|
||||
assert await rl.get_workflow_slot_mapping(run_id_2) == (
|
||||
11,
|
||||
"slot-2",
|
||||
None,
|
||||
)
|
||||
finally:
|
||||
await redis_client.delete(f"workflow_slot_mapping:{run_id_2}")
|
||||
finally:
|
||||
await redis_client.delete(mapping_key)
|
||||
await rl.close()
|
||||
|
|
@ -25,6 +25,7 @@ from api.db.models import (
|
|||
WorkflowModel,
|
||||
WorkflowRunModel,
|
||||
)
|
||||
from api.services.call_concurrency import CallConcurrencySlot
|
||||
from api.services.campaign.campaign_call_dispatcher import CampaignCallDispatcher
|
||||
|
||||
# =============================================================================
|
||||
|
|
@ -259,6 +260,27 @@ def mock_rate_limiter():
|
|||
}
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def mock_call_concurrency():
|
||||
async def acquire_slot(organization_id, *, source, **kwargs):
|
||||
return CallConcurrencySlot(
|
||||
organization_id=organization_id,
|
||||
slot_id=f"slot-{uuid.uuid4().hex[:8]}",
|
||||
max_concurrent=20,
|
||||
source=source,
|
||||
scope_key=kwargs.get("scope_key"),
|
||||
)
|
||||
|
||||
with patch(
|
||||
"api.services.campaign.campaign_call_dispatcher.call_concurrency"
|
||||
) as mock_concurrency:
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(side_effect=acquire_slot)
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock(return_value=True)
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock(return_value=True)
|
||||
yield mock_concurrency
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Tests
|
||||
# =============================================================================
|
||||
|
|
@ -793,3 +815,47 @@ class TestProcessBatchEdgeCases:
|
|||
{"campaign_id": campaign_test_data.campaign_id},
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
|
||||
class TestAcquireConcurrentSlotScoping:
|
||||
"""Campaign max_concurrency must scope to the campaign, not the org counter."""
|
||||
|
||||
def _campaign(self, orchestrator_metadata):
|
||||
campaign = MagicMock()
|
||||
campaign.id = 42
|
||||
campaign.orchestrator_metadata = orchestrator_metadata
|
||||
return campaign
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_campaign_max_concurrency_uses_campaign_scope(
|
||||
self, mock_call_concurrency
|
||||
):
|
||||
dispatcher = CampaignCallDispatcher()
|
||||
campaign = self._campaign({"max_concurrency": 3})
|
||||
|
||||
await dispatcher.acquire_concurrent_slot(7, campaign, timeout=5)
|
||||
|
||||
mock_call_concurrency.acquire_org_slot.assert_awaited_once_with(
|
||||
7,
|
||||
source="campaign:42",
|
||||
timeout=5,
|
||||
scope_key="campaign:42",
|
||||
scope_max_concurrent=3,
|
||||
retry_interval=1,
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_campaign_max_concurrency_skips_scope(self, mock_call_concurrency):
|
||||
dispatcher = CampaignCallDispatcher()
|
||||
campaign = self._campaign({})
|
||||
|
||||
await dispatcher.acquire_concurrent_slot(7, campaign, timeout=5)
|
||||
|
||||
mock_call_concurrency.acquire_org_slot.assert_awaited_once_with(
|
||||
7,
|
||||
source="campaign:42",
|
||||
timeout=5,
|
||||
scope_key=None,
|
||||
scope_max_concurrent=None,
|
||||
retry_interval=1,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -677,15 +677,20 @@ class TestProcessStatusUpdateCircuitBreaker:
|
|||
|
||||
with (
|
||||
patch("api.services.telephony.status_processor.db_client") as mock_db,
|
||||
patch(
|
||||
"api.services.telephony.status_processor.campaign_call_dispatcher"
|
||||
) as mock_dispatcher,
|
||||
patch("api.services.telephony.status_processor.circuit_breaker") as mock_cb,
|
||||
):
|
||||
mock_db.get_workflow_run_by_id = AsyncMock(return_value=mock_workflow_run)
|
||||
mock_db.update_workflow_run = AsyncMock()
|
||||
mock_dispatcher.release_call_slot = AsyncMock(return_value=True)
|
||||
|
||||
await _process_status_update(100, status)
|
||||
|
||||
# Circuit breaker should NOT be called for non-campaign calls
|
||||
mock_cb.record_and_evaluate.assert_not_called()
|
||||
mock_dispatcher.release_call_slot.assert_awaited_once_with(100)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
|||
|
||||
import pytest
|
||||
|
||||
from api.services.call_concurrency import CallConcurrencySlot
|
||||
from api.services.campaign.campaign_call_dispatcher import CampaignCallDispatcher
|
||||
from api.services.campaign.rate_limiter import RateLimiter
|
||||
|
||||
|
|
@ -266,6 +267,9 @@ class TestDispatcherThreadsTelephonyConfig:
|
|||
patch(
|
||||
"api.services.campaign.campaign_call_dispatcher.rate_limiter"
|
||||
) as mock_rl,
|
||||
patch(
|
||||
"api.services.campaign.campaign_call_dispatcher.call_concurrency"
|
||||
) as mock_concurrency,
|
||||
patch(
|
||||
"api.services.campaign.campaign_call_dispatcher.get_backend_endpoints",
|
||||
AsyncMock(return_value=("https://example.com", None)),
|
||||
|
|
@ -280,14 +284,21 @@ class TestDispatcherThreadsTelephonyConfig:
|
|||
mock_db.get_workflow_by_id = AsyncMock(return_value=SimpleNamespace(id=1))
|
||||
mock_db.create_workflow_run = AsyncMock(return_value=workflow_run)
|
||||
mock_db.update_workflow_run = AsyncMock()
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
|
||||
mock_rl.acquire_from_number = AsyncMock(return_value="+15551110001")
|
||||
mock_rl.release_from_number = AsyncMock()
|
||||
mock_rl.release_concurrent_slot = AsyncMock()
|
||||
mock_rl.store_workflow_slot_mapping = AsyncMock()
|
||||
mock_rl.store_workflow_from_number_mapping = AsyncMock()
|
||||
|
||||
await dispatcher.dispatch_call(queued_run, campaign, slot_id="slot-1")
|
||||
slot = CallConcurrencySlot(
|
||||
organization_id=org_id,
|
||||
slot_id="slot-1",
|
||||
max_concurrent=1,
|
||||
source="test",
|
||||
)
|
||||
await dispatcher.dispatch_call(queued_run, campaign, slot)
|
||||
|
||||
# acquire_from_number on rate_limiter must be called with the
|
||||
# campaign's telephony_configuration_id.
|
||||
|
|
@ -337,10 +348,15 @@ class TestDispatcherThreadsTelephonyConfig:
|
|||
|
||||
dispatcher = CampaignCallDispatcher()
|
||||
|
||||
with patch(
|
||||
"api.services.campaign.campaign_call_dispatcher.rate_limiter"
|
||||
) as mock_rl:
|
||||
mock_rl.get_workflow_slot_mapping = AsyncMock(return_value=None)
|
||||
with (
|
||||
patch(
|
||||
"api.services.campaign.campaign_call_dispatcher.rate_limiter"
|
||||
) as mock_rl,
|
||||
patch(
|
||||
"api.services.campaign.campaign_call_dispatcher.call_concurrency"
|
||||
) as mock_concurrency,
|
||||
):
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock(return_value=False)
|
||||
mock_rl.get_workflow_from_number_mapping = AsyncMock(
|
||||
return_value=(org_id, from_number, config_id)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ from fastapi import FastAPI
|
|||
from fastapi.testclient import TestClient
|
||||
|
||||
from api.routes.public_agent import router
|
||||
from api.services.call_concurrency import CallConcurrencyLimitError
|
||||
|
||||
|
||||
def _make_test_app() -> FastAPI:
|
||||
|
|
@ -56,6 +57,7 @@ def test_trigger_route_executes_as_workflow_owner():
|
|||
|
||||
with (
|
||||
patch("api.routes.public_agent.db_client") as mock_db,
|
||||
patch("api.routes.public_agent.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.public_agent.authorize_workflow_run_start",
|
||||
new=quota_mock,
|
||||
|
|
@ -69,6 +71,12 @@ def test_trigger_route_executes_as_workflow_owner():
|
|||
new=AsyncMock(return_value=("https://api.example.com", "wss://ignored")),
|
||||
),
|
||||
):
|
||||
slot = object()
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(return_value=slot)
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
|
||||
mock_db.validate_api_key = AsyncMock(
|
||||
return_value=SimpleNamespace(id=7, organization_id=11, created_by=22)
|
||||
)
|
||||
|
|
@ -96,6 +104,12 @@ def test_trigger_route_executes_as_workflow_owner():
|
|||
workflow_id=workflow.id,
|
||||
workflow_run_id=501,
|
||||
)
|
||||
mock_concurrency.acquire_org_slot.assert_awaited_once_with(
|
||||
workflow.organization_id,
|
||||
source="public_agent",
|
||||
timeout=0,
|
||||
)
|
||||
mock_concurrency.bind_workflow_run.assert_awaited_once_with(slot, 501)
|
||||
mock_db.get_workflow.assert_awaited_once_with(workflow.id, organization_id=11)
|
||||
|
||||
create_kwargs = mock_db.create_workflow_run.await_args.kwargs
|
||||
|
|
@ -126,6 +140,7 @@ def test_workflow_uuid_route_uses_scoped_lookup_and_shared_execution():
|
|||
|
||||
with (
|
||||
patch("api.routes.public_agent.db_client") as mock_db,
|
||||
patch("api.routes.public_agent.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.public_agent.authorize_workflow_run_start",
|
||||
new=quota_mock,
|
||||
|
|
@ -139,6 +154,12 @@ def test_workflow_uuid_route_uses_scoped_lookup_and_shared_execution():
|
|||
new=AsyncMock(return_value=("https://api.example.com", "wss://ignored")),
|
||||
),
|
||||
):
|
||||
slot = object()
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(return_value=slot)
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
|
||||
mock_db.validate_api_key = AsyncMock(
|
||||
return_value=SimpleNamespace(id=8, organization_id=11, created_by=22)
|
||||
)
|
||||
|
|
@ -160,6 +181,12 @@ def test_workflow_uuid_route_uses_scoped_lookup_and_shared_execution():
|
|||
11,
|
||||
)
|
||||
assert not mock_db.get_agent_trigger_by_path.called
|
||||
mock_concurrency.acquire_org_slot.assert_awaited_once_with(
|
||||
workflow.organization_id,
|
||||
source="public_agent",
|
||||
timeout=0,
|
||||
)
|
||||
mock_concurrency.bind_workflow_run.assert_awaited_once_with(slot, 601)
|
||||
|
||||
create_kwargs = mock_db.create_workflow_run.await_args.kwargs
|
||||
assert create_kwargs["user_id"] == workflow.user_id
|
||||
|
|
@ -170,6 +197,110 @@ def test_workflow_uuid_route_uses_scoped_lookup_and_shared_execution():
|
|||
assert "agent_uuid" not in create_kwargs["initial_context"]
|
||||
|
||||
|
||||
def test_trigger_route_rejects_when_concurrency_limit_reached():
|
||||
app = _make_test_app()
|
||||
client = TestClient(app)
|
||||
|
||||
workflow = _active_workflow(trigger_path="trigger-uuid-123")
|
||||
provider = _provider()
|
||||
|
||||
with (
|
||||
patch("api.routes.public_agent.db_client") as mock_db,
|
||||
patch("api.routes.public_agent.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.public_agent.get_default_telephony_provider",
|
||||
new=AsyncMock(return_value=provider),
|
||||
),
|
||||
):
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(
|
||||
side_effect=CallConcurrencyLimitError(
|
||||
organization_id=11,
|
||||
source="public_agent",
|
||||
wait_time=0,
|
||||
max_concurrent=2,
|
||||
)
|
||||
)
|
||||
mock_db.validate_api_key = AsyncMock(
|
||||
return_value=SimpleNamespace(id=7, organization_id=11, created_by=22)
|
||||
)
|
||||
mock_db.get_agent_trigger_by_path = AsyncMock(
|
||||
return_value=SimpleNamespace(
|
||||
workflow_id=workflow.id,
|
||||
organization_id=11,
|
||||
state="active",
|
||||
)
|
||||
)
|
||||
mock_db.get_workflow = AsyncMock(return_value=workflow)
|
||||
mock_db.get_default_telephony_configuration = AsyncMock(
|
||||
return_value=SimpleNamespace(id=55)
|
||||
)
|
||||
mock_db.create_workflow_run = AsyncMock()
|
||||
|
||||
response = client.post(
|
||||
"/public/agent/trigger-uuid-123",
|
||||
headers={"X-API-Key": "test-api-key"},
|
||||
json={"phone_number": "+15551234567"},
|
||||
)
|
||||
|
||||
assert response.status_code == 429
|
||||
assert response.json()["detail"] == "Concurrent call limit reached"
|
||||
mock_db.create_workflow_run.assert_not_called()
|
||||
|
||||
|
||||
def test_trigger_route_releases_concurrency_slot_when_quota_fails():
|
||||
app = _make_test_app()
|
||||
client = TestClient(app)
|
||||
|
||||
workflow = _active_workflow(trigger_path="trigger-uuid-123")
|
||||
provider = _provider()
|
||||
quota_mock = AsyncMock(
|
||||
return_value=SimpleNamespace(has_quota=False, error_message="Quota exceeded")
|
||||
)
|
||||
|
||||
with (
|
||||
patch("api.routes.public_agent.db_client") as mock_db,
|
||||
patch("api.routes.public_agent.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.public_agent.authorize_workflow_run_start",
|
||||
new=quota_mock,
|
||||
),
|
||||
patch(
|
||||
"api.routes.public_agent.get_default_telephony_provider",
|
||||
new=AsyncMock(return_value=provider),
|
||||
),
|
||||
):
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(return_value=object())
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
|
||||
mock_db.validate_api_key = AsyncMock(
|
||||
return_value=SimpleNamespace(id=7, organization_id=11, created_by=22)
|
||||
)
|
||||
mock_db.get_agent_trigger_by_path = AsyncMock(
|
||||
return_value=SimpleNamespace(
|
||||
workflow_id=workflow.id,
|
||||
organization_id=11,
|
||||
state="active",
|
||||
)
|
||||
)
|
||||
mock_db.get_workflow = AsyncMock(return_value=workflow)
|
||||
mock_db.get_default_telephony_configuration = AsyncMock(
|
||||
return_value=SimpleNamespace(id=55)
|
||||
)
|
||||
mock_db.create_workflow_run = AsyncMock(return_value=SimpleNamespace(id=501))
|
||||
|
||||
response = client.post(
|
||||
"/public/agent/trigger-uuid-123",
|
||||
headers={"X-API-Key": "test-api-key"},
|
||||
json={"phone_number": "+15551234567"},
|
||||
)
|
||||
|
||||
assert response.status_code == 402
|
||||
mock_concurrency.release_workflow_run_slot.assert_awaited_once_with(501)
|
||||
provider.initiate_call.assert_not_awaited()
|
||||
|
||||
|
||||
def test_workflow_uuid_route_rejects_archived_workflows():
|
||||
app = _make_test_app()
|
||||
client = TestClient(app)
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ _ACTIVE_TOKEN = SimpleNamespace(
|
|||
expires_at=None,
|
||||
allowed_domains=[],
|
||||
workflow_id=1,
|
||||
organization_id=11,
|
||||
created_by=7,
|
||||
usage_limit=None,
|
||||
usage_count=0,
|
||||
|
|
@ -37,6 +38,7 @@ _RESTRICTED_TOKEN = SimpleNamespace(
|
|||
expires_at=None,
|
||||
allowed_domains=["allowed.example.com"],
|
||||
workflow_id=2,
|
||||
organization_id=11,
|
||||
created_by=7,
|
||||
usage_limit=None,
|
||||
usage_count=0,
|
||||
|
|
@ -49,6 +51,7 @@ _LOCALHOST_TOKEN = SimpleNamespace(
|
|||
expires_at=None,
|
||||
allowed_domains=["localhost:3000", "localhost:3020"],
|
||||
workflow_id=3,
|
||||
organization_id=11,
|
||||
created_by=7,
|
||||
usage_limit=None,
|
||||
usage_count=0,
|
||||
|
|
|
|||
|
|
@ -6,8 +6,10 @@ from fastapi import FastAPI
|
|||
from fastapi.testclient import TestClient
|
||||
|
||||
from api.enums import WorkflowRunMode, WorkflowRunState
|
||||
from api.routes.telephony import _handle_telephony_websocket, router
|
||||
from api.errors.telephony_errors import TelephonyError
|
||||
from api.routes.telephony import _handle_telephony_websocket, handle_inbound_run, router
|
||||
from api.services.auth.depends import get_user
|
||||
from api.services.call_concurrency import CallConcurrencyLimitError
|
||||
|
||||
|
||||
def _make_test_app() -> FastAPI:
|
||||
|
|
@ -55,6 +57,7 @@ def test_initiate_call_executes_as_workflow_owner_for_shared_org_workflow():
|
|||
|
||||
with (
|
||||
patch("api.routes.telephony.db_client") as mock_db,
|
||||
patch("api.routes.telephony.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.telephony.authorize_workflow_run_start",
|
||||
new=quota_mock,
|
||||
|
|
@ -68,6 +71,12 @@ def test_initiate_call_executes_as_workflow_owner_for_shared_org_workflow():
|
|||
new=AsyncMock(return_value=("https://api.example.com", "wss://ignored")),
|
||||
),
|
||||
):
|
||||
slot = object()
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(return_value=slot)
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
|
||||
mock_db.get_user_configurations = AsyncMock(
|
||||
return_value=SimpleNamespace(test_phone_number=None)
|
||||
)
|
||||
|
|
@ -104,6 +113,12 @@ def test_initiate_call_executes_as_workflow_owner_for_shared_org_workflow():
|
|||
assert create_kwargs["user_id"] == workflow.user_id
|
||||
assert create_kwargs["organization_id"] == workflow.organization_id
|
||||
assert create_kwargs["initial_context"]["template_key"] == "template-value"
|
||||
mock_concurrency.acquire_org_slot.assert_awaited_once_with(
|
||||
workflow.organization_id,
|
||||
source="telephony_outbound",
|
||||
timeout=0,
|
||||
)
|
||||
mock_concurrency.bind_workflow_run.assert_awaited_once_with(slot, 501)
|
||||
|
||||
initiate_kwargs = provider.initiate_call.await_args.kwargs
|
||||
assert initiate_kwargs["workflow_id"] == workflow.id
|
||||
|
|
@ -124,6 +139,7 @@ def test_initiate_call_uses_organization_preference_phone_number():
|
|||
|
||||
with (
|
||||
patch("api.routes.telephony.db_client") as mock_db,
|
||||
patch("api.routes.telephony.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.telephony.authorize_workflow_run_start",
|
||||
new=quota_mock,
|
||||
|
|
@ -137,6 +153,11 @@ def test_initiate_call_uses_organization_preference_phone_number():
|
|||
new=AsyncMock(return_value=("https://api.example.com", "wss://ignored")),
|
||||
),
|
||||
):
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(return_value=object())
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
|
||||
mock_db.get_user_configurations = AsyncMock(
|
||||
return_value=SimpleNamespace(test_phone_number="+15550000000")
|
||||
)
|
||||
|
|
@ -178,6 +199,7 @@ def test_initiate_call_rejects_existing_run_for_different_workflow():
|
|||
|
||||
with (
|
||||
patch("api.routes.telephony.db_client") as mock_db,
|
||||
patch("api.routes.telephony.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.telephony.authorize_workflow_run_start",
|
||||
new=quota_mock,
|
||||
|
|
@ -187,6 +209,11 @@ def test_initiate_call_rejects_existing_run_for_different_workflow():
|
|||
new=AsyncMock(return_value=provider),
|
||||
),
|
||||
):
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(return_value=object())
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
mock_concurrency.release_slot = AsyncMock()
|
||||
mock_concurrency.release_workflow_run_slot = AsyncMock()
|
||||
|
||||
mock_db.get_user_configurations = AsyncMock(
|
||||
return_value=SimpleNamespace(test_phone_number=None)
|
||||
)
|
||||
|
|
@ -215,10 +242,119 @@ def test_initiate_call_rejects_existing_run_for_different_workflow():
|
|||
assert response.status_code == 400
|
||||
assert response.json()["detail"] == "workflow_run_workflow_mismatch"
|
||||
mock_db.get_workflow_run.assert_awaited_once_with(501, organization_id=11)
|
||||
mock_concurrency.release_slot.assert_awaited_once()
|
||||
assert not mock_db.create_workflow_run.called
|
||||
assert provider.initiate_call.await_count == 0
|
||||
|
||||
|
||||
def test_initiate_call_rejects_when_concurrency_limit_reached():
|
||||
app = _make_test_app()
|
||||
client = TestClient(app)
|
||||
|
||||
workflow = _workflow()
|
||||
provider = _provider()
|
||||
|
||||
with (
|
||||
patch("api.routes.telephony.db_client") as mock_db,
|
||||
patch("api.routes.telephony.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.telephony.get_default_telephony_provider",
|
||||
new=AsyncMock(return_value=provider),
|
||||
),
|
||||
):
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(
|
||||
side_effect=CallConcurrencyLimitError(
|
||||
organization_id=workflow.organization_id,
|
||||
source="telephony_outbound",
|
||||
wait_time=0,
|
||||
max_concurrent=1,
|
||||
)
|
||||
)
|
||||
mock_db.get_default_telephony_configuration = AsyncMock(
|
||||
return_value=SimpleNamespace(id=55)
|
||||
)
|
||||
mock_db.get_workflow = AsyncMock(return_value=workflow)
|
||||
mock_db.create_workflow_run = AsyncMock()
|
||||
|
||||
response = client.post(
|
||||
"/telephony/initiate-call",
|
||||
json={"workflow_id": workflow.id, "phone_number": "+15551234567"},
|
||||
)
|
||||
|
||||
assert response.status_code == 429
|
||||
assert response.json()["detail"] == "Concurrent call limit reached"
|
||||
mock_db.create_workflow_run.assert_not_called()
|
||||
provider.initiate_call.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_inbound_run_rejects_when_concurrency_limit_reached():
|
||||
request = SimpleNamespace(headers={}, url="https://api.example.com/inbound/run")
|
||||
provider_class = SimpleNamespace(
|
||||
PROVIDER_NAME="twilio",
|
||||
generate_validation_error_response=Mock(return_value="limit-response"),
|
||||
)
|
||||
normalized_data = SimpleNamespace(
|
||||
provider="twilio",
|
||||
direction="inbound",
|
||||
to_number="+15551230000",
|
||||
from_number="+15557650000",
|
||||
to_country="US",
|
||||
from_country="US",
|
||||
account_id="acct-1",
|
||||
call_id="call-1",
|
||||
raw_data={},
|
||||
)
|
||||
config = SimpleNamespace(id=55, organization_id=11)
|
||||
phone_row = SimpleNamespace(id=77, inbound_workflow_id=33)
|
||||
workflow = SimpleNamespace(id=33, user_id=99)
|
||||
provider_instance = SimpleNamespace(
|
||||
verify_inbound_signature=AsyncMock(return_value=True)
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"api.routes.telephony.parse_webhook_request",
|
||||
new=AsyncMock(return_value=({}, "raw-body")),
|
||||
),
|
||||
patch(
|
||||
"api.routes.telephony._detect_provider",
|
||||
new=AsyncMock(return_value=provider_class),
|
||||
),
|
||||
patch(
|
||||
"api.routes.telephony.normalize_webhook_data",
|
||||
return_value=normalized_data,
|
||||
),
|
||||
patch("api.routes.telephony.db_client") as mock_db,
|
||||
patch(
|
||||
"api.routes.telephony.get_telephony_provider_by_id",
|
||||
new=AsyncMock(return_value=provider_instance),
|
||||
),
|
||||
patch("api.routes.telephony.call_concurrency") as mock_concurrency,
|
||||
):
|
||||
mock_db.find_inbound_route_by_account = AsyncMock(
|
||||
return_value=(config, phone_row)
|
||||
)
|
||||
mock_db.get_workflow = AsyncMock(return_value=workflow)
|
||||
mock_db.create_workflow_run = AsyncMock()
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(
|
||||
side_effect=CallConcurrencyLimitError(
|
||||
organization_id=config.organization_id,
|
||||
source="inbound:twilio",
|
||||
wait_time=0,
|
||||
max_concurrent=1,
|
||||
)
|
||||
)
|
||||
|
||||
response = await handle_inbound_run(request)
|
||||
|
||||
assert response == "limit-response"
|
||||
provider_class.generate_validation_error_response.assert_called_once_with(
|
||||
TelephonyError.CONCURRENT_CALL_LIMIT
|
||||
)
|
||||
mock_db.create_workflow_run.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_smallwebrtc_run_reaching_telephony_websocket_closes_without_running():
|
||||
websocket = AsyncMock()
|
||||
|
|
@ -235,11 +371,13 @@ async def test_smallwebrtc_run_reaching_telephony_websocket_closes_without_runni
|
|||
|
||||
with (
|
||||
patch("api.routes.telephony.db_client") as mock_db,
|
||||
patch("api.routes.telephony.call_concurrency") as mock_concurrency,
|
||||
patch(
|
||||
"api.routes.telephony.get_telephony_provider_for_run",
|
||||
new=provider_lookup,
|
||||
),
|
||||
):
|
||||
mock_concurrency.unregister_active_call = AsyncMock()
|
||||
mock_db.get_workflow_run = AsyncMock(return_value=workflow_run)
|
||||
mock_db.get_workflow_by_id = AsyncMock(return_value=workflow)
|
||||
mock_db.update_workflow_run = AsyncMock()
|
||||
|
|
@ -255,3 +393,4 @@ async def test_smallwebrtc_run_reaching_telephony_websocket_closes_without_runni
|
|||
)
|
||||
assert mock_db.update_workflow_run.await_count == 0
|
||||
assert provider_lookup.await_count == 0
|
||||
mock_concurrency.unregister_active_call.assert_not_awaited()
|
||||
|
|
|
|||
167
api/tests/test_webrtc_signaling_concurrency.py
Normal file
167
api/tests/test_webrtc_signaling_concurrency.py
Normal file
|
|
@ -0,0 +1,167 @@
|
|||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from api.routes.webrtc_signaling import SignalingManager
|
||||
from api.services.call_concurrency import CallConcurrencyLimitError
|
||||
|
||||
|
||||
class _FakeWebSocket:
|
||||
def __init__(self):
|
||||
self.send_json = AsyncMock()
|
||||
|
||||
|
||||
class _FakePeerConnection:
|
||||
def __init__(self):
|
||||
self.renegotiate = AsyncMock()
|
||||
|
||||
def get_answer(self):
|
||||
return {"sdp": "v=0\r\n", "type": "answer", "pc_id": "pc-1"}
|
||||
|
||||
|
||||
def _offer_payload(pc_id: str = "pc-1") -> dict:
|
||||
return {
|
||||
"pc_id": pc_id,
|
||||
"sdp": "v=0\r\n",
|
||||
"type": "offer",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_public_embed_offer_rejects_when_org_concurrency_limit_reached():
|
||||
manager = SignalingManager()
|
||||
ws = _FakeWebSocket()
|
||||
user = SimpleNamespace(id=7)
|
||||
|
||||
with (
|
||||
patch("api.routes.webrtc_signaling.db_client") as mock_db,
|
||||
patch(
|
||||
"api.routes.webrtc_signaling.authorize_workflow_run_start",
|
||||
new=AsyncMock(
|
||||
return_value=SimpleNamespace(has_quota=True, error_message="")
|
||||
),
|
||||
),
|
||||
patch("api.routes.webrtc_signaling.call_concurrency") as mock_concurrency,
|
||||
):
|
||||
mock_db.get_workflow_organization_id = AsyncMock(return_value=11)
|
||||
mock_concurrency.acquire_org_slot = AsyncMock(
|
||||
side_effect=CallConcurrencyLimitError(
|
||||
organization_id=11,
|
||||
source="public_embed",
|
||||
wait_time=0,
|
||||
max_concurrent=2,
|
||||
)
|
||||
)
|
||||
mock_concurrency.bind_workflow_run = AsyncMock()
|
||||
|
||||
await manager._handle_offer(
|
||||
ws,
|
||||
_offer_payload(),
|
||||
workflow_id=33,
|
||||
workflow_run_id=501,
|
||||
user=user,
|
||||
connection_key="conn-1",
|
||||
enforce_call_concurrency=True,
|
||||
call_concurrency_source="public_embed",
|
||||
)
|
||||
|
||||
ws.send_json.assert_awaited_once_with(
|
||||
{
|
||||
"type": "error",
|
||||
"payload": {
|
||||
"error_type": "concurrency_limit_exceeded",
|
||||
"message": "Concurrent call limit reached",
|
||||
},
|
||||
}
|
||||
)
|
||||
mock_concurrency.bind_workflow_run.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_public_embed_renegotiation_does_not_acquire_another_slot():
|
||||
manager = SignalingManager()
|
||||
ws = _FakeWebSocket()
|
||||
user = SimpleNamespace(id=7)
|
||||
connection_key = "conn-1"
|
||||
pc = _FakePeerConnection()
|
||||
manager._peer_connections["pc-1"] = pc
|
||||
manager._peer_connection_owners["pc-1"] = connection_key
|
||||
|
||||
with (
|
||||
patch("api.routes.webrtc_signaling.db_client") as mock_db,
|
||||
patch(
|
||||
"api.routes.webrtc_signaling.authorize_workflow_run_start",
|
||||
new=AsyncMock(
|
||||
return_value=SimpleNamespace(has_quota=True, error_message="")
|
||||
),
|
||||
),
|
||||
patch("api.routes.webrtc_signaling.call_concurrency") as mock_concurrency,
|
||||
):
|
||||
mock_db.get_workflow_organization_id = AsyncMock(return_value=11)
|
||||
mock_concurrency.acquire_org_slot = AsyncMock()
|
||||
|
||||
await manager._handle_offer(
|
||||
ws,
|
||||
_offer_payload(),
|
||||
workflow_id=33,
|
||||
workflow_run_id=501,
|
||||
user=user,
|
||||
connection_key=connection_key,
|
||||
enforce_call_concurrency=True,
|
||||
call_concurrency_source="public_embed",
|
||||
)
|
||||
|
||||
mock_concurrency.acquire_org_slot.assert_not_called()
|
||||
pc.renegotiate.assert_awaited_once()
|
||||
assert ws.send_json.await_args.args[0]["type"] == "answer"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_signaling_websocket_rejects_run_not_owned_by_workflow():
|
||||
"""The URL workflow_id drives org/quota/concurrency accounting, so a
|
||||
workflow_id that doesn't own the run must be rejected before signaling."""
|
||||
from fastapi import HTTPException
|
||||
|
||||
from api.routes.webrtc_signaling import signaling_websocket
|
||||
|
||||
ws = _FakeWebSocket()
|
||||
user = SimpleNamespace(id=7)
|
||||
|
||||
with (
|
||||
patch("api.routes.webrtc_signaling.db_client") as mock_db,
|
||||
patch("api.routes.webrtc_signaling.signaling_manager") as mock_manager,
|
||||
):
|
||||
mock_db.get_workflow_run = AsyncMock(
|
||||
return_value=SimpleNamespace(id=501, workflow_id=99)
|
||||
)
|
||||
mock_manager.handle_websocket = AsyncMock()
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await signaling_websocket(
|
||||
ws, workflow_id=33, workflow_run_id=501, user=user
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
mock_manager.handle_websocket.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_signaling_websocket_accepts_matching_workflow_and_run():
|
||||
from api.routes.webrtc_signaling import signaling_websocket
|
||||
|
||||
ws = _FakeWebSocket()
|
||||
user = SimpleNamespace(id=7)
|
||||
|
||||
with (
|
||||
patch("api.routes.webrtc_signaling.db_client") as mock_db,
|
||||
patch("api.routes.webrtc_signaling.signaling_manager") as mock_manager,
|
||||
):
|
||||
mock_db.get_workflow_run = AsyncMock(
|
||||
return_value=SimpleNamespace(id=501, workflow_id=33)
|
||||
)
|
||||
mock_manager.handle_websocket = AsyncMock()
|
||||
|
||||
await signaling_websocket(ws, workflow_id=33, workflow_run_id=501, user=user)
|
||||
|
||||
mock_manager.handle_websocket.assert_awaited_once()
|
||||
61
api/tests/test_workflow_configurations_schema.py
Normal file
61
api/tests/test_workflow_configurations_schema.py
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from api.schemas.workflow_configurations import (
|
||||
DEFAULT_MAX_CALL_DURATION_SECONDS,
|
||||
MAX_CALL_DURATION_SECONDS,
|
||||
WorkflowConfigurationDefaults,
|
||||
)
|
||||
|
||||
|
||||
def test_max_call_duration_default_within_bounds():
|
||||
config = WorkflowConfigurationDefaults()
|
||||
assert config.max_call_duration == DEFAULT_MAX_CALL_DURATION_SECONDS
|
||||
|
||||
|
||||
def test_max_call_duration_accepts_cap():
|
||||
config = WorkflowConfigurationDefaults(max_call_duration=MAX_CALL_DURATION_SECONDS)
|
||||
assert config.max_call_duration == MAX_CALL_DURATION_SECONDS
|
||||
|
||||
|
||||
def test_max_call_duration_rejects_over_cap():
|
||||
with pytest.raises(ValidationError):
|
||||
WorkflowConfigurationDefaults(max_call_duration=MAX_CALL_DURATION_SECONDS + 1)
|
||||
|
||||
|
||||
def test_max_call_duration_rejects_non_positive():
|
||||
with pytest.raises(ValidationError):
|
||||
WorkflowConfigurationDefaults(max_call_duration=0)
|
||||
|
||||
|
||||
def test_null_values_treated_as_unset():
|
||||
"""Stored configs / older clients send explicit JSON nulls for keys the
|
||||
user never configured; they must validate as defaults, not fail."""
|
||||
config = WorkflowConfigurationDefaults.model_validate(
|
||||
{
|
||||
"max_call_duration": None,
|
||||
"turn_start_strategy": None,
|
||||
"turn_start_min_words": None,
|
||||
}
|
||||
)
|
||||
assert config.max_call_duration == DEFAULT_MAX_CALL_DURATION_SECONDS
|
||||
# Nulls count as unset, so a sparse round-trip drops them entirely.
|
||||
assert config.model_dump(exclude_unset=True) == {}
|
||||
|
||||
|
||||
def test_exclude_unset_round_trip_stays_sparse():
|
||||
config = WorkflowConfigurationDefaults.model_validate(
|
||||
{"max_call_duration": 600, "custom_extra_key": {"a": 1}}
|
||||
)
|
||||
assert config.model_dump(exclude_unset=True) == {
|
||||
"max_call_duration": 600,
|
||||
"custom_extra_key": {"a": 1},
|
||||
}
|
||||
|
||||
|
||||
def test_cap_stays_within_concurrency_stale_timeout():
|
||||
"""A call outliving the rate limiter's stale window has its concurrency
|
||||
slot purged mid-call, so the cap must never exceed it."""
|
||||
from api.services.campaign.rate_limiter import rate_limiter
|
||||
|
||||
assert MAX_CALL_DURATION_SECONDS <= rate_limiter.stale_call_timeout
|
||||
Loading…
Add table
Add a link
Reference in a new issue