Skip to content

Commit

Permalink
Fix forward to no route targets
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
garw committed Aug 17, 2023
1 parent deeb389 commit 055cf5c
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 12 deletions.
35 changes: 33 additions & 2 deletions tests/test_ywsd_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ 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():
if key == "members":
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:
Expand Down Expand Up @@ -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"
25 changes: 25 additions & 0 deletions tests/testdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
]
)
)
Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = py38,py39,py310
envlist = py38,py39,py310,py311
skip_missing_interpreters = true


Expand All @@ -11,4 +11,4 @@ deps =
pytest-black
pytest-asyncio
docker
commands = pytest --black {posargs}
commands = pytest --black {posargs}
19 changes: 11 additions & 8 deletions ywsd/routing_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 055cf5c

Please sign in to comment.