Skip to content

Commit 20c94df

Browse files
author
Suresh Kumar Moharajan
committed
add invalid query to get_server , list_servers and list_servers_for_user improve test coverage
Signed-off-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
1 parent d3122e5 commit 20c94df

2 files changed

Lines changed: 181 additions & 1 deletion

File tree

mcpgateway/services/server_service.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,13 +815,18 @@ async def list_servers(
815815
return (cached_servers, cached.get("next_cursor"))
816816

817817
# Build base query with ordering and eager load relationships to avoid N+1
818+
# Filter out deactivated tools, resources, prompts, and agents at query level
818819
query = (
819820
select(DbServer)
820821
.options(
821822
selectinload(DbServer.tools),
823+
with_loader_criteria(DbTool, DbTool.enabled.is_(True)),
822824
selectinload(DbServer.resources),
825+
with_loader_criteria(DbResource, DbResource.enabled.is_(True)),
823826
selectinload(DbServer.prompts),
827+
with_loader_criteria(DbPrompt, DbPrompt.enabled.is_(True)),
824828
selectinload(DbServer.a2a_agents),
829+
with_loader_criteria(DbA2AAgent, DbA2AAgent.enabled.is_(True)),
825830
joinedload(DbServer.email_team),
826831
)
827832
.order_by(desc(DbServer.created_at), desc(DbServer.id))
@@ -924,11 +929,16 @@ async def list_servers_for_user(
924929
team_ids = [team.id for team in user_teams]
925930

926931
# Eager load relationships to avoid N+1 queries
932+
# Filter out deactivated tools, resources, prompts, and agents at query level
927933
query = select(DbServer).options(
928934
selectinload(DbServer.tools),
935+
with_loader_criteria(DbTool, DbTool.enabled.is_(True)),
929936
selectinload(DbServer.resources),
937+
with_loader_criteria(DbResource, DbResource.enabled.is_(True)),
930938
selectinload(DbServer.prompts),
939+
with_loader_criteria(DbPrompt, DbPrompt.enabled.is_(True)),
931940
selectinload(DbServer.a2a_agents),
941+
with_loader_criteria(DbA2AAgent, DbA2AAgent.enabled.is_(True)),
932942
joinedload(DbServer.email_team),
933943
)
934944

tests/unit/mcpgateway/services/test_server_service.py

Lines changed: 171 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3571,4 +3571,174 @@ async def test_get_server_filters_deactivated_entities(self, server_service, tes
35713571

35723572
assert len(result.associated_prompts) == 1
35733573
assert "prompt-1" in result.associated_prompts
3574-
assert "prompt-2" not in result.associated_prompts
3574+
3575+
@pytest.mark.asyncio
3576+
async def test_list_servers_filters_deactivated_entities(self, server_service, test_db):
3577+
"""Test that list_servers filters out deactivated tools, prompts, and resources."""
3578+
# Standard
3579+
from types import SimpleNamespace
3580+
from unittest.mock import AsyncMock
3581+
3582+
# Create mock tools - one enabled, one disabled
3583+
enabled_tool = SimpleNamespace(id="tool-1", name="enabled_tool", enabled=True)
3584+
disabled_tool = SimpleNamespace(id="tool-2", name="disabled_tool", enabled=False)
3585+
3586+
# Create mock resources - one enabled, one disabled
3587+
enabled_resource = SimpleNamespace(id="res-1", name="enabled_resource", enabled=True)
3588+
disabled_resource = SimpleNamespace(id="res-2", name="disabled_resource", enabled=False)
3589+
3590+
# Create mock prompts - one enabled, one disabled
3591+
enabled_prompt = SimpleNamespace(id="prompt-1", name="enabled_prompt", enabled=True)
3592+
disabled_prompt = SimpleNamespace(id="prompt-2", name="disabled_prompt", enabled=False)
3593+
3594+
# Create mock server with both enabled and disabled entities
3595+
# Due to with_loader_criteria(), only enabled entities should be loaded
3596+
mock_server = SimpleNamespace(
3597+
id="server-1",
3598+
name="test_server",
3599+
description="Test server",
3600+
icon=None,
3601+
created_at="2023-01-01T00:00:00",
3602+
updated_at="2023-01-01T00:00:00",
3603+
enabled=True,
3604+
team_id=None,
3605+
owner_email="user@example.com",
3606+
visibility="public",
3607+
created_by="user@example.com",
3608+
modified_by="user@example.com",
3609+
created_from_ip=None,
3610+
created_via=None,
3611+
created_user_agent=None,
3612+
modified_from_ip=None,
3613+
modified_via=None,
3614+
modified_user_agent=None,
3615+
import_batch_id=None,
3616+
federation_source=None,
3617+
version=1,
3618+
oauth_enabled=False,
3619+
oauth_config=None,
3620+
tags=[],
3621+
# Only enabled entities should be loaded due to with_loader_criteria()
3622+
tools=[enabled_tool],
3623+
resources=[enabled_resource],
3624+
prompts=[enabled_prompt],
3625+
a2a_agents=[],
3626+
metrics=[],
3627+
email_team=None,
3628+
team=None,
3629+
)
3630+
3631+
# Mock unified_paginate to return our server
3632+
# Note: list_servers uses cursor-based pagination by default (page=None),
3633+
# so unified_paginate should return a tuple (servers_list, next_cursor)
3634+
with patch("mcpgateway.services.server_service.unified_paginate", new_callable=AsyncMock) as mock_paginate:
3635+
mock_paginate.return_value = ([mock_server], None)
3636+
3637+
# Call list_servers
3638+
servers, next_cursor = await server_service.list_servers(test_db, include_inactive=False)
3639+
3640+
# Verify that only enabled entities are in the result
3641+
assert len(servers) == 1
3642+
server_result = servers[0]
3643+
3644+
assert len(server_result.associated_tools) == 1
3645+
assert "enabled_tool" in server_result.associated_tools
3646+
assert "disabled_tool" not in server_result.associated_tools
3647+
3648+
assert len(server_result.associated_resources) == 1
3649+
assert "res-1" in server_result.associated_resources
3650+
assert "res-2" not in server_result.associated_resources
3651+
3652+
assert len(server_result.associated_prompts) == 1
3653+
assert "prompt-1" in server_result.associated_prompts
3654+
assert "prompt-2" not in server_result.associated_prompts
3655+
3656+
@pytest.mark.asyncio
3657+
async def test_list_servers_for_user_filters_deactivated_entities(self, server_service, test_db):
3658+
"""Test that list_servers_for_user filters out deactivated tools, prompts, and resources."""
3659+
# Standard
3660+
from types import SimpleNamespace
3661+
from unittest.mock import AsyncMock
3662+
3663+
# Create mock tools - one enabled, one disabled
3664+
enabled_tool = SimpleNamespace(id="tool-1", name="enabled_tool", enabled=True)
3665+
disabled_tool = SimpleNamespace(id="tool-2", name="disabled_tool", enabled=False)
3666+
3667+
# Create mock resources - one enabled, one disabled
3668+
enabled_resource = SimpleNamespace(id="res-1", name="enabled_resource", enabled=True)
3669+
disabled_resource = SimpleNamespace(id="res-2", name="disabled_resource", enabled=False)
3670+
3671+
# Create mock prompts - one enabled, one disabled
3672+
enabled_prompt = SimpleNamespace(id="prompt-1", name="enabled_prompt", enabled=True)
3673+
disabled_prompt = SimpleNamespace(id="prompt-2", name="disabled_prompt", enabled=False)
3674+
3675+
# Create mock server with both enabled and disabled entities
3676+
# Due to with_loader_criteria(), only enabled entities should be loaded
3677+
mock_server = SimpleNamespace(
3678+
id="server-1",
3679+
name="test_server",
3680+
description="Test server",
3681+
icon=None,
3682+
created_at="2023-01-01T00:00:00",
3683+
updated_at="2023-01-01T00:00:00",
3684+
enabled=True,
3685+
team_id=None,
3686+
owner_email="user@example.com",
3687+
visibility="public",
3688+
created_by="user@example.com",
3689+
modified_by="user@example.com",
3690+
created_from_ip=None,
3691+
created_via=None,
3692+
created_user_agent=None,
3693+
modified_from_ip=None,
3694+
modified_via=None,
3695+
modified_user_agent=None,
3696+
import_batch_id=None,
3697+
federation_source=None,
3698+
version=1,
3699+
oauth_enabled=False,
3700+
oauth_config=None,
3701+
tags=[],
3702+
# Only enabled entities should be loaded due to with_loader_criteria()
3703+
tools=[enabled_tool],
3704+
resources=[enabled_resource],
3705+
prompts=[enabled_prompt],
3706+
a2a_agents=[],
3707+
metrics=[],
3708+
email_team=None,
3709+
team=None,
3710+
)
3711+
3712+
# Mock TeamManagementService
3713+
with patch("mcpgateway.services.server_service.TeamManagementService") as mock_team_service_class:
3714+
mock_team_service = Mock()
3715+
mock_team_service.get_user_teams = AsyncMock(return_value=[])
3716+
mock_team_service_class.return_value = mock_team_service
3717+
3718+
# Mock the database execute to return our server
3719+
mock_result = Mock()
3720+
mock_result.scalars = Mock(return_value=Mock(all=Mock(return_value=[mock_server])))
3721+
test_db.execute = Mock(return_value=mock_result)
3722+
3723+
# Call list_servers_for_user
3724+
servers = await server_service.list_servers_for_user(
3725+
test_db,
3726+
user_email="user@example.com",
3727+
include_inactive=False
3728+
)
3729+
3730+
# Verify that only enabled entities are in the result
3731+
assert len(servers) == 1
3732+
server_result = servers[0]
3733+
3734+
assert len(server_result.associated_tools) == 1
3735+
assert "enabled_tool" in server_result.associated_tools
3736+
assert "disabled_tool" not in server_result.associated_tools
3737+
3738+
assert len(server_result.associated_resources) == 1
3739+
assert "res-1" in server_result.associated_resources
3740+
assert "res-2" not in server_result.associated_resources
3741+
3742+
assert len(server_result.associated_prompts) == 1
3743+
assert "prompt-1" in server_result.associated_prompts
3744+
assert "prompt-2" not in server_result.associated_prompts

0 commit comments

Comments
 (0)