From 055cf5cbd4434843c149a554c7e726575b6f4c91 Mon Sep 17 00:00:00 2001 From: Martin Lang Date: Thu, 17 Aug 2023 11:29:41 +0200 Subject: [PATCH] Fix forward to no route targets Delayed forwards cause another fork rank to be created. However, it might be that the forward target isn't reachable and has no route. In this case, the forward should be dropped and not added as fork rank. This commit fixes this and adds a regression test. --- tests/test_ywsd_web.py | 35 +++++++++++++++++++++++++++++++++-- tests/testdata.py | 25 +++++++++++++++++++++++++ tox.ini | 4 ++-- ywsd/routing_tree.py | 19 +++++++++++-------- 4 files changed, 71 insertions(+), 12 deletions(-) diff --git a/tests/test_ywsd_web.py b/tests/test_ywsd_web.py index 62ba8eb..e16984e 100644 --- a/tests/test_ywsd_web.py +++ b/tests/test_ywsd_web.py @@ -33,7 +33,7 @@ def validate_routing_tree(observed, expected): assert ("fork_ranks" in observed) == ("fork_ranks" in expected) if "fork_ranks" in observed: - for (observed_rank, expected_rank) in zip( + for observed_rank, expected_rank in zip( observed["fork_ranks"], expected["fork_ranks"] ): for key in expected_rank.keys(): @@ -41,7 +41,7 @@ def validate_routing_tree(observed, expected): continue assert observed_rank[key] == expected_rank[key] assert len(observed_rank["members"]) == len(expected_rank["members"]) - for (observed_member, expected_member) in zip( + for observed_member, expected_member in zip( observed_rank["members"], expected_rank["members"] ): if "type" in expected_member: @@ -307,3 +307,34 @@ async def test_webserver_immediate_forward_4748_to_2098(ywsd_engine_web): } validate_routing_tree(data["routing_tree"], expected_routing_tree) assert data["routing_tree"]["forwarding_extension"]["extension"] == "2005" + + +@pytest.mark.asyncio +@pytest.mark.usefixtures("ywsd_engine_web") +async def test_webserver_delayed_forward_to_empty_group_4000(ywsd_engine_web): + async with aiohttp.ClientSession() as session: + async with session.get( + "http://localhost:9042/stage1?caller=4748&called=4001" + ) as response: + data = await response.json() + print(repr(data)) + assert data["routing_status"] == "OK" + main_routing_result = data["main_routing_result"] + assert main_routing_result["type"] == "Type.FORK" + expected_fork_targets = [ + { + "target": "lateroute/4001", + "parameters": {"eventphone_stage2": "1"}, + }, + ] + validate_fork_targets( + main_routing_result["fork_targets"], expected_fork_targets + ) + expected_routing_tree = { + "extension": "4001", + "type": "Type.SIMPLE", + "forwarding_mode": "ForwardingMode.ENABLED", + "forwarding_delay": 10, + } + validate_routing_tree(data["routing_tree"], expected_routing_tree) + assert data["routing_tree"]["forwarding_extension"]["extension"] == "4000" diff --git a/tests/testdata.py b/tests/testdata.py index 2c311fa..4d34899 100644 --- a/tests/testdata.py +++ b/tests/testdata.py @@ -116,6 +116,26 @@ async def write_testdata(conn): "lang": "de_DE", "ringback": None, }, + { + "yate_id": None, + "extension": "4000", + "name": "Empty Group", + "type": "GROUP", + "forwarding_mode": "DISABLED", + "forwarding_delay": None, + "lang": "de_DE", + "ringback": None, + }, + { + "yate_id": yates["sip"], + "extension": "4001", + "name": "I Forward to empty group", + "type": "SIMPLE", + "forwarding_mode": "DISABLED", + "forwarding_delay": 10, + "lang": "de_DE", + "ringback": None, + }, ] ) ) @@ -134,6 +154,11 @@ async def write_testdata(conn): .where(Extension.table.c.extension == "2098") .values({"forwarding_extension_id": exts["2005"], "forwarding_mode": "ENABLED"}) ) + await conn.execute( + Extension.table.update() + .where(Extension.table.c.extension == "4001") + .values({"forwarding_extension_id": exts["4000"], "forwarding_mode": "ENABLED"}) + ) await conn.execute( ForkRank.table.insert().values( diff --git a/tox.ini b/tox.ini index b2398a1..2725a36 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py38,py39,py310 +envlist = py38,py39,py310,py311 skip_missing_interpreters = true @@ -11,4 +11,4 @@ deps = pytest-black pytest-asyncio docker -commands = pytest --black {posargs} \ No newline at end of file +commands = pytest --black {posargs} diff --git a/ywsd/routing_tree.py b/ywsd/routing_tree.py index dc88152..b7c6088 100644 --- a/ywsd/routing_tree.py +++ b/ywsd/routing_tree.py @@ -461,14 +461,17 @@ def _visit_for_route_calculation( ): # this is non-immediate forward forwarding_route = self._visit(node.forwarding_extension, local_path) - if node.forwarding_mode == Extension.ForwardingMode.ENABLED: - fwd_delay = node.forwarding_delay - accumulated_delay - fork_targets.append(CallTarget("|drop={}".format(fwd_delay))) - else: - # Add a default rank, call will progress to next rang when all previous calls failed - fork_targets.append(CallTarget("|")) - fork_targets.append(forwarding_route.target) - self._cache_intermediate_result(forwarding_route) + # if we obtained a valid route, we add this as new fork target here. Otherwise, we keep + # the current fork ranks and just move on. + if forwarding_route.is_valid: + if node.forwarding_mode == Extension.ForwardingMode.ENABLED: + fwd_delay = node.forwarding_delay - accumulated_delay + fork_targets.append(CallTarget("|drop={}".format(fwd_delay))) + else: + # Add a default rank, call will progress to next rang when all previous calls failed + fork_targets.append(CallTarget("|")) + fork_targets.append(forwarding_route.target) + self._cache_intermediate_result(forwarding_route) return self._make_intermediate_result( fork_targets=fork_targets,