mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
feat: add /clear endpoint for V1 conversations (#12786)
Co-authored-by: mkdev11 <MkDev11@users.noreply.github.com> Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: tofarr <tofarr@gmail.com> Co-authored-by: hieptl <hieptl.developer@gmail.com>
This commit is contained in:
@@ -286,6 +286,54 @@ class TestSQLAppConversationInfoService:
|
||||
results = await service.batch_get_app_conversation_info([])
|
||||
assert results == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_count_conversations_by_sandbox_id(
|
||||
self,
|
||||
service: SQLAppConversationInfoService,
|
||||
):
|
||||
"""Test count by sandbox_id: only delete sandbox when no conversation uses it."""
|
||||
base_time = datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
|
||||
shared_sandbox = 'shared_sandbox_1'
|
||||
other_sandbox = 'other_sandbox'
|
||||
for i in range(3):
|
||||
info = AppConversationInfo(
|
||||
id=uuid4(),
|
||||
created_by_user_id=None,
|
||||
sandbox_id=shared_sandbox,
|
||||
selected_repository='https://github.com/test/repo',
|
||||
selected_branch='main',
|
||||
git_provider=ProviderType.GITHUB,
|
||||
title=f'Conversation {i}',
|
||||
trigger=ConversationTrigger.GUI,
|
||||
pr_number=[],
|
||||
llm_model='gpt-4',
|
||||
metrics=None,
|
||||
created_at=base_time,
|
||||
updated_at=base_time,
|
||||
)
|
||||
await service.save_app_conversation_info(info)
|
||||
for i in range(2):
|
||||
info = AppConversationInfo(
|
||||
id=uuid4(),
|
||||
created_by_user_id=None,
|
||||
sandbox_id=other_sandbox,
|
||||
selected_repository='https://github.com/test/repo',
|
||||
selected_branch='main',
|
||||
git_provider=ProviderType.GITHUB,
|
||||
title=f'Other {i}',
|
||||
trigger=ConversationTrigger.GUI,
|
||||
pr_number=[],
|
||||
llm_model='gpt-4',
|
||||
metrics=None,
|
||||
created_at=base_time,
|
||||
updated_at=base_time,
|
||||
)
|
||||
await service.save_app_conversation_info(info)
|
||||
|
||||
assert await service.count_conversations_by_sandbox_id(shared_sandbox) == 3
|
||||
assert await service.count_conversations_by_sandbox_id(other_sandbox) == 2
|
||||
assert await service.count_conversations_by_sandbox_id('no_such_sandbox') == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_search_conversation_info_no_filters(
|
||||
self,
|
||||
|
||||
@@ -1038,6 +1038,9 @@ async def test_delete_v1_conversation_success():
|
||||
return_value=mock_app_conversation_info
|
||||
)
|
||||
mock_service.delete_app_conversation = AsyncMock(return_value=True)
|
||||
mock_info_service.count_conversations_by_sandbox_id = AsyncMock(
|
||||
return_value=1
|
||||
)
|
||||
|
||||
# Call delete_conversation with V1 conversation ID
|
||||
result = await delete_conversation(
|
||||
@@ -1059,7 +1062,8 @@ async def test_delete_v1_conversation_success():
|
||||
|
||||
# Verify that delete_app_conversation was called with the conversation ID
|
||||
mock_service.delete_app_conversation.assert_called_once_with(
|
||||
conversation_uuid
|
||||
conversation_uuid,
|
||||
skip_agent_server_delete=False,
|
||||
)
|
||||
|
||||
|
||||
@@ -1357,6 +1361,9 @@ async def test_delete_v1_conversation_with_agent_server():
|
||||
return_value=mock_app_conversation_info
|
||||
)
|
||||
mock_service.delete_app_conversation = AsyncMock(return_value=True)
|
||||
mock_info_service.count_conversations_by_sandbox_id = AsyncMock(
|
||||
return_value=1
|
||||
)
|
||||
|
||||
# Call delete_conversation with V1 conversation ID
|
||||
result = await delete_conversation(
|
||||
@@ -1378,7 +1385,8 @@ async def test_delete_v1_conversation_with_agent_server():
|
||||
|
||||
# Verify that delete_app_conversation was called with the conversation ID
|
||||
mock_service.delete_app_conversation.assert_called_once_with(
|
||||
conversation_uuid
|
||||
conversation_uuid,
|
||||
skip_agent_server_delete=False,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
@@ -21,34 +20,46 @@ def app():
|
||||
return app
|
||||
|
||||
|
||||
def test_localhost_cors_middleware_init_with_env_var():
|
||||
"""Test that the middleware correctly parses PERMITTED_CORS_ORIGINS environment variable."""
|
||||
with patch.dict(
|
||||
os.environ, {'PERMITTED_CORS_ORIGINS': 'https://example.com,https://test.com'}
|
||||
def test_localhost_cors_middleware_init_with_config():
|
||||
"""Test that the middleware correctly reads permitted_cors_origins from global config."""
|
||||
mock_config = MagicMock()
|
||||
mock_config.permitted_cors_origins = [
|
||||
'https://example.com',
|
||||
'https://test.com',
|
||||
]
|
||||
with patch(
|
||||
'openhands.server.middleware.get_global_config', return_value=mock_config
|
||||
):
|
||||
app = FastAPI()
|
||||
middleware = LocalhostCORSMiddleware(app)
|
||||
|
||||
# Check that the origins were correctly parsed from the environment variable
|
||||
# Check that the origins were correctly read from the config
|
||||
assert 'https://example.com' in middleware.allow_origins
|
||||
assert 'https://test.com' in middleware.allow_origins
|
||||
assert len(middleware.allow_origins) == 2
|
||||
|
||||
|
||||
def test_localhost_cors_middleware_init_without_env_var():
|
||||
"""Test that the middleware works correctly without PERMITTED_CORS_ORIGINS environment variable."""
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
def test_localhost_cors_middleware_init_without_config():
|
||||
"""Test that the middleware works correctly without permitted_cors_origins configured."""
|
||||
mock_config = MagicMock()
|
||||
mock_config.permitted_cors_origins = []
|
||||
with patch(
|
||||
'openhands.server.middleware.get_global_config', return_value=mock_config
|
||||
):
|
||||
app = FastAPI()
|
||||
middleware = LocalhostCORSMiddleware(app)
|
||||
|
||||
# Check that allow_origins is empty when no environment variable is set
|
||||
# Check that allow_origins is empty when no origins are configured
|
||||
assert middleware.allow_origins == ()
|
||||
|
||||
|
||||
def test_localhost_cors_middleware_is_allowed_origin_localhost(app):
|
||||
"""Test that localhost origins are allowed regardless of port when no specific origins are configured."""
|
||||
# Test without setting PERMITTED_CORS_ORIGINS to trigger localhost behavior
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
mock_config = MagicMock()
|
||||
mock_config.permitted_cors_origins = []
|
||||
with patch(
|
||||
'openhands.server.middleware.get_global_config', return_value=mock_config
|
||||
):
|
||||
app.add_middleware(LocalhostCORSMiddleware)
|
||||
client = TestClient(app)
|
||||
|
||||
@@ -76,8 +87,11 @@ def test_localhost_cors_middleware_is_allowed_origin_localhost(app):
|
||||
|
||||
def test_localhost_cors_middleware_is_allowed_origin_non_localhost(app):
|
||||
"""Test that non-localhost origins follow the standard CORS rules."""
|
||||
# Set up the middleware with specific allowed origins
|
||||
with patch.dict(os.environ, {'PERMITTED_CORS_ORIGINS': 'https://example.com'}):
|
||||
mock_config = MagicMock()
|
||||
mock_config.permitted_cors_origins = ['https://example.com']
|
||||
with patch(
|
||||
'openhands.server.middleware.get_global_config', return_value=mock_config
|
||||
):
|
||||
app.add_middleware(LocalhostCORSMiddleware)
|
||||
client = TestClient(app)
|
||||
|
||||
@@ -95,7 +109,11 @@ def test_localhost_cors_middleware_is_allowed_origin_non_localhost(app):
|
||||
|
||||
def test_localhost_cors_middleware_missing_origin(app):
|
||||
"""Test behavior when Origin header is missing."""
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
mock_config = MagicMock()
|
||||
mock_config.permitted_cors_origins = []
|
||||
with patch(
|
||||
'openhands.server.middleware.get_global_config', return_value=mock_config
|
||||
):
|
||||
app.add_middleware(LocalhostCORSMiddleware)
|
||||
client = TestClient(app)
|
||||
|
||||
@@ -113,17 +131,22 @@ def test_localhost_cors_middleware_inheritance():
|
||||
|
||||
def test_localhost_cors_middleware_cors_parameters():
|
||||
"""Test that CORS parameters are set correctly in the middleware."""
|
||||
# We need to inspect the initialization parameters rather than attributes
|
||||
# since CORSMiddleware doesn't expose these as attributes
|
||||
with patch('fastapi.middleware.cors.CORSMiddleware.__init__') as mock_init:
|
||||
mock_init.return_value = None
|
||||
app = FastAPI()
|
||||
LocalhostCORSMiddleware(app)
|
||||
mock_config = MagicMock()
|
||||
mock_config.permitted_cors_origins = []
|
||||
with patch(
|
||||
'openhands.server.middleware.get_global_config', return_value=mock_config
|
||||
):
|
||||
# We need to inspect the initialization parameters rather than attributes
|
||||
# since CORSMiddleware doesn't expose these as attributes
|
||||
with patch('fastapi.middleware.cors.CORSMiddleware.__init__') as mock_init:
|
||||
mock_init.return_value = None
|
||||
app = FastAPI()
|
||||
LocalhostCORSMiddleware(app)
|
||||
|
||||
# Check that the parent class was initialized with the correct parameters
|
||||
mock_init.assert_called_once()
|
||||
_, kwargs = mock_init.call_args
|
||||
# Check that the parent class was initialized with the correct parameters
|
||||
mock_init.assert_called_once()
|
||||
_, kwargs = mock_init.call_args
|
||||
|
||||
assert kwargs['allow_credentials'] is True
|
||||
assert kwargs['allow_methods'] == ['*']
|
||||
assert kwargs['allow_headers'] == ['*']
|
||||
assert kwargs['allow_credentials'] is True
|
||||
assert kwargs['allow_methods'] == ['*']
|
||||
assert kwargs['allow_headers'] == ['*']
|
||||
|
||||
Reference in New Issue
Block a user