Skip to content

Commit e190c86

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 59cd81b commit e190c86

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
@@ -791,13 +791,18 @@ async def list_servers(
791791
return (cached_servers, cached.get("next_cursor"))
792792

793793
# Build base query with ordering and eager load relationships to avoid N+1
794+
# Filter out deactivated tools, resources, prompts, and agents at query level
794795
query = (
795796
select(DbServer)
796797
.options(
797798
selectinload(DbServer.tools),
799+
with_loader_criteria(DbTool, DbTool.enabled.is_(True)),
798800
selectinload(DbServer.resources),
801+
with_loader_criteria(DbResource, DbResource.enabled.is_(True)),
799802
selectinload(DbServer.prompts),
803+
with_loader_criteria(DbPrompt, DbPrompt.enabled.is_(True)),
800804
selectinload(DbServer.a2a_agents),
805+
with_loader_criteria(DbA2AAgent, DbA2AAgent.enabled.is_(True)),
801806
joinedload(DbServer.email_team),
802807
)
803808
.order_by(desc(DbServer.created_at), desc(DbServer.id))
@@ -904,11 +909,16 @@ async def list_servers_for_user(
904909
team_ids = [team.id for team in user_teams]
905910

906911
# Eager load relationships to avoid N+1 queries
912+
# Filter out deactivated tools, resources, prompts, and agents at query level
907913
query = select(DbServer).options(
908914
selectinload(DbServer.tools),
915+
with_loader_criteria(DbTool, DbTool.enabled.is_(True)),
909916
selectinload(DbServer.resources),
917+
with_loader_criteria(DbResource, DbResource.enabled.is_(True)),
910918
selectinload(DbServer.prompts),
919+
with_loader_criteria(DbPrompt, DbPrompt.enabled.is_(True)),
911920
selectinload(DbServer.a2a_agents),
921+
with_loader_criteria(DbA2AAgent, DbA2AAgent.enabled.is_(True)),
912922
joinedload(DbServer.email_team),
913923
)
914924

tests/unit/mcpgateway/services/test_server_service.py

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

36423642
assert len(result.associated_prompts) == 1
36433643
assert "prompt-1" in result.associated_prompts
3644-
assert "prompt-2" not in result.associated_prompts
3644+
3645+
@pytest.mark.asyncio
3646+
async def test_list_servers_filters_deactivated_entities(self, server_service, test_db):
3647+
"""Test that list_servers filters out deactivated tools, prompts, and resources."""
3648+
# Standard
3649+
from types import SimpleNamespace
3650+
from unittest.mock import AsyncMock
3651+
3652+
# Create mock tools - one enabled, one disabled
3653+
enabled_tool = SimpleNamespace(id="tool-1", name="enabled_tool", enabled=True)
3654+
disabled_tool = SimpleNamespace(id="tool-2", name="disabled_tool", enabled=False)
3655+
3656+
# Create mock resources - one enabled, one disabled
3657+
enabled_resource = SimpleNamespace(id="res-1", name="enabled_resource", enabled=True)
3658+
disabled_resource = SimpleNamespace(id="res-2", name="disabled_resource", enabled=False)
3659+
3660+
# Create mock prompts - one enabled, one disabled
3661+
enabled_prompt = SimpleNamespace(id="prompt-1", name="enabled_prompt", enabled=True)
3662+
disabled_prompt = SimpleNamespace(id="prompt-2", name="disabled_prompt", enabled=False)
3663+
3664+
# Create mock server with both enabled and disabled entities
3665+
# Due to with_loader_criteria(), only enabled entities should be loaded
3666+
mock_server = SimpleNamespace(
3667+
id="server-1",
3668+
name="test_server",
3669+
description="Test server",
3670+
icon=None,
3671+
created_at="2023-01-01T00:00:00",
3672+
updated_at="2023-01-01T00:00:00",
3673+
enabled=True,
3674+
team_id=None,
3675+
owner_email="user@example.com",
3676+
visibility="public",
3677+
created_by="user@example.com",
3678+
modified_by="user@example.com",
3679+
created_from_ip=None,
3680+
created_via=None,
3681+
created_user_agent=None,
3682+
modified_from_ip=None,
3683+
modified_via=None,
3684+
modified_user_agent=None,
3685+
import_batch_id=None,
3686+
federation_source=None,
3687+
version=1,
3688+
oauth_enabled=False,
3689+
oauth_config=None,
3690+
tags=[],
3691+
# Only enabled entities should be loaded due to with_loader_criteria()
3692+
tools=[enabled_tool],
3693+
resources=[enabled_resource],
3694+
prompts=[enabled_prompt],
3695+
a2a_agents=[],
3696+
metrics=[],
3697+
email_team=None,
3698+
team=None,
3699+
)
3700+
3701+
# Mock unified_paginate to return our server
3702+
# Note: list_servers uses cursor-based pagination by default (page=None),
3703+
# so unified_paginate should return a tuple (servers_list, next_cursor)
3704+
with patch("mcpgateway.services.server_service.unified_paginate", new_callable=AsyncMock) as mock_paginate:
3705+
mock_paginate.return_value = ([mock_server], None)
3706+
3707+
# Call list_servers
3708+
servers, next_cursor = await server_service.list_servers(test_db, include_inactive=False)
3709+
3710+
# Verify that only enabled entities are in the result
3711+
assert len(servers) == 1
3712+
server_result = servers[0]
3713+
3714+
assert len(server_result.associated_tools) == 1
3715+
assert "enabled_tool" in server_result.associated_tools
3716+
assert "disabled_tool" not in server_result.associated_tools
3717+
3718+
assert len(server_result.associated_resources) == 1
3719+
assert "res-1" in server_result.associated_resources
3720+
assert "res-2" not in server_result.associated_resources
3721+
3722+
assert len(server_result.associated_prompts) == 1
3723+
assert "prompt-1" in server_result.associated_prompts
3724+
assert "prompt-2" not in server_result.associated_prompts
3725+
3726+
@pytest.mark.asyncio
3727+
async def test_list_servers_for_user_filters_deactivated_entities(self, server_service, test_db):
3728+
"""Test that list_servers_for_user filters out deactivated tools, prompts, and resources."""
3729+
# Standard
3730+
from types import SimpleNamespace
3731+
from unittest.mock import AsyncMock
3732+
3733+
# Create mock tools - one enabled, one disabled
3734+
enabled_tool = SimpleNamespace(id="tool-1", name="enabled_tool", enabled=True)
3735+
disabled_tool = SimpleNamespace(id="tool-2", name="disabled_tool", enabled=False)
3736+
3737+
# Create mock resources - one enabled, one disabled
3738+
enabled_resource = SimpleNamespace(id="res-1", name="enabled_resource", enabled=True)
3739+
disabled_resource = SimpleNamespace(id="res-2", name="disabled_resource", enabled=False)
3740+
3741+
# Create mock prompts - one enabled, one disabled
3742+
enabled_prompt = SimpleNamespace(id="prompt-1", name="enabled_prompt", enabled=True)
3743+
disabled_prompt = SimpleNamespace(id="prompt-2", name="disabled_prompt", enabled=False)
3744+
3745+
# Create mock server with both enabled and disabled entities
3746+
# Due to with_loader_criteria(), only enabled entities should be loaded
3747+
mock_server = SimpleNamespace(
3748+
id="server-1",
3749+
name="test_server",
3750+
description="Test server",
3751+
icon=None,
3752+
created_at="2023-01-01T00:00:00",
3753+
updated_at="2023-01-01T00:00:00",
3754+
enabled=True,
3755+
team_id=None,
3756+
owner_email="user@example.com",
3757+
visibility="public",
3758+
created_by="user@example.com",
3759+
modified_by="user@example.com",
3760+
created_from_ip=None,
3761+
created_via=None,
3762+
created_user_agent=None,
3763+
modified_from_ip=None,
3764+
modified_via=None,
3765+
modified_user_agent=None,
3766+
import_batch_id=None,
3767+
federation_source=None,
3768+
version=1,
3769+
oauth_enabled=False,
3770+
oauth_config=None,
3771+
tags=[],
3772+
# Only enabled entities should be loaded due to with_loader_criteria()
3773+
tools=[enabled_tool],
3774+
resources=[enabled_resource],
3775+
prompts=[enabled_prompt],
3776+
a2a_agents=[],
3777+
metrics=[],
3778+
email_team=None,
3779+
team=None,
3780+
)
3781+
3782+
# Mock TeamManagementService
3783+
with patch("mcpgateway.services.server_service.TeamManagementService") as mock_team_service_class:
3784+
mock_team_service = Mock()
3785+
mock_team_service.get_user_teams = AsyncMock(return_value=[])
3786+
mock_team_service_class.return_value = mock_team_service
3787+
3788+
# Mock the database execute to return our server
3789+
mock_result = Mock()
3790+
mock_result.scalars = Mock(return_value=Mock(all=Mock(return_value=[mock_server])))
3791+
test_db.execute = Mock(return_value=mock_result)
3792+
3793+
# Call list_servers_for_user
3794+
servers = await server_service.list_servers_for_user(
3795+
test_db,
3796+
user_email="user@example.com",
3797+
include_inactive=False
3798+
)
3799+
3800+
# Verify that only enabled entities are in the result
3801+
assert len(servers) == 1
3802+
server_result = servers[0]
3803+
3804+
assert len(server_result.associated_tools) == 1
3805+
assert "enabled_tool" in server_result.associated_tools
3806+
assert "disabled_tool" not in server_result.associated_tools
3807+
3808+
assert len(server_result.associated_resources) == 1
3809+
assert "res-1" in server_result.associated_resources
3810+
assert "res-2" not in server_result.associated_resources
3811+
3812+
assert len(server_result.associated_prompts) == 1
3813+
assert "prompt-1" in server_result.associated_prompts
3814+
assert "prompt-2" not in server_result.associated_prompts

0 commit comments

Comments
 (0)