admin/dify
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

refactor: port reqparse to BaseModel (#28993)

Browse Source 
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit is contained in:
Asuka Minato
2025-12-08 15:31:19 +09:00
committed by GitHub
parent 2f96374837
commit 05fe92a541
44 changed files with 1531 additions and 1894 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
api/tests/unit_tests/controllers/console/billing/test_billing.py
View File

@@ -125,7 +125,7 @@ class TestPartnerTenants:
resource = PartnerTenants()
# Act & Assert
# reqparse will raise BadRequest for missing required field
# Validation should raise BadRequest for missing required field
with pytest.raises(BadRequest):
resource.put(partner_key_encoded)

26
api/tests/unit_tests/controllers/service_api/app/test_file_preview.py
View File

@@ -256,24 +256,18 @@ class TestFilePreviewApi:
mock_app, # App query for tenant validation
]
with patch("controllers.service_api.app.file_preview.reqparse") as mock_reqparse:
# Mock request parsing
mock_parser = Mock()
mock_parser.parse_args.return_value = {"as_attachment": False}
mock_reqparse.RequestParser.return_value = mock_parser
# Test the core logic directly without Flask decorators
# Validate file ownership
result_message_file, result_upload_file = file_preview_api._validate_file_ownership(file_id, app_id)
assert result_message_file == mock_message_file
assert result_upload_file == mock_upload_file
# Test the core logic directly without Flask decorators
# Validate file ownership
result_message_file, result_upload_file = file_preview_api._validate_file_ownership(file_id, app_id)
assert result_message_file == mock_message_file
assert result_upload_file == mock_upload_file
# Test file response building
response = file_preview_api._build_file_response(mock_generator, mock_upload_file, False)
assert response is not None
# Test file response building
response = file_preview_api._build_file_response(mock_generator, mock_upload_file, False)
assert response is not None
# Verify storage was called correctly
mock_storage.load.assert_not_called() # Since we're testing components separately
# Verify storage was called correctly
mock_storage.load.assert_not_called() # Since we're testing components separately
@patch("controllers.service_api.app.file_preview.storage")
def test_storage_error_handling(

75
api/tests/unit_tests/services/test_metadata_bug_complete.py
View File

@@ -2,8 +2,6 @@ from pathlib import Path
from unittest.mock import Mock, create_autospec, patch
import pytest
from flask_restx import reqparse
from werkzeug.exceptions import BadRequest
from models.account import Account
from services.entities.knowledge_entities.knowledge_entities import MetadataArgs
@@ -77,60 +75,39 @@ class TestMetadataBugCompleteValidation:
assert type_column.nullable is False, "type column should be nullable=False"
assert name_column.nullable is False, "name column should be nullable=False"
def test_4_fixed_api_layer_rejects_null(self, app):
"""Test Layer 4: Fixed API configuration properly rejects null values."""
# Test Console API create endpoint (fixed)
parser = (
reqparse.RequestParser()
.add_argument("type", type=str, required=True, nullable=False, location="json")
.add_argument("name", type=str, required=True, nullable=False, location="json")
)
def test_4_fixed_api_layer_rejects_null(self):
"""Test Layer 4: Fixed API configuration properly rejects null values using Pydantic."""
with pytest.raises((ValueError, TypeError)):
MetadataArgs.model_validate({"type": None, "name": None})
with app.test_request_context(json={"type": None, "name": None}, content_type="application/json"):
with pytest.raises(BadRequest):
parser.parse_args()
with pytest.raises((ValueError, TypeError)):
MetadataArgs.model_validate({"type": "string", "name": None})
# Test with just name being null
with app.test_request_context(json={"type": "string", "name": None}, content_type="application/json"):
with pytest.raises(BadRequest):
parser.parse_args()
with pytest.raises((ValueError, TypeError)):
MetadataArgs.model_validate({"type": None, "name": "test"})
# Test with just type being null
with app.test_request_context(json={"type": None, "name": "test"}, content_type="application/json"):
with pytest.raises(BadRequest):
parser.parse_args()
def test_5_fixed_api_accepts_valid_values(self, app):
def test_5_fixed_api_accepts_valid_values(self):
"""Test that fixed API still accepts valid non-null values."""
parser = (
reqparse.RequestParser()
.add_argument("type", type=str, required=True, nullable=False, location="json")
.add_argument("name", type=str, required=True, nullable=False, location="json")
)
args = MetadataArgs.model_validate({"type": "string", "name": "valid_name"})
assert args.type == "string"
assert args.name == "valid_name"
with app.test_request_context(json={"type": "string", "name": "valid_name"}, content_type="application/json"):
args = parser.parse_args()
assert args["type"] == "string"
assert args["name"] == "valid_name"
def test_6_simulated_buggy_behavior(self):
"""Test simulating the original buggy behavior by bypassing Pydantic validation."""
mock_metadata_args = Mock()
mock_metadata_args.name = None
mock_metadata_args.type = None
def test_6_simulated_buggy_behavior(self, app):
"""Test simulating the original buggy behavior with nullable=True."""
# Simulate the old buggy configuration
buggy_parser = (
reqparse.RequestParser()
.add_argument("type", type=str, required=True, nullable=True, location="json")
.add_argument("name", type=str, required=True, nullable=True, location="json")
)
mock_user = create_autospec(Account, instance=True)
mock_user.current_tenant_id = "tenant-123"
mock_user.id = "user-456"
with app.test_request_context(json={"type": None, "name": None}, content_type="application/json"):
# This would pass in the buggy version
args = buggy_parser.parse_args()
assert args["type"] is None
assert args["name"] is None
# But would crash when trying to create MetadataArgs
with pytest.raises((ValueError, TypeError)):
MetadataArgs.model_validate(args)
with patch(
"services.metadata_service.current_account_with_tenant",
return_value=(mock_user, mock_user.current_tenant_id),
):
with pytest.raises(TypeError, match="object of type 'NoneType' has no len"):
MetadataService.create_metadata("dataset-123", mock_metadata_args)
def test_7_end_to_end_validation_layers(self):
"""Test all validation layers work together correctly."""

79
api/tests/unit_tests/services/test_metadata_nullable_bug.py
View File

@@ -1,7 +1,6 @@
from unittest.mock import Mock, create_autospec, patch
import pytest
from flask_restx import reqparse
from models.account import Account
from services.entities.knowledge_entities.knowledge_entities import MetadataArgs
@@ -51,76 +50,16 @@ class TestMetadataNullableBug:
with pytest.raises(TypeError, match="object of type 'NoneType' has no len"):
MetadataService.update_metadata_name("dataset-123", "metadata-456", None)
def test_api_parser_accepts_null_values(self, app):
"""Test that API parser configuration incorrectly accepts null values."""
# Simulate the current API parser configuration
parser = (
reqparse.RequestParser()
.add_argument("type", type=str, required=True, nullable=True, location="json")
.add_argument("name", type=str, required=True, nullable=True, location="json")
)
def test_api_layer_now_uses_pydantic_validation(self):
"""Verify that API layer relies on Pydantic validation instead of reqparse."""
invalid_payload = {"type": None, "name": None}
with pytest.raises((ValueError, TypeError)):
MetadataArgs.model_validate(invalid_payload)
# Simulate request data with null values
with app.test_request_context(json={"type": None, "name": None}, content_type="application/json"):
# This should parse successfully due to nullable=True
args = parser.parse_args()
# Verify that null values are accepted
assert args["type"] is None
assert args["name"] is None
# This demonstrates the bug: API accepts None but business logic will crash
def test_integration_bug_scenario(self, app):
"""Test the complete bug scenario from API to service layer."""
# Step 1: API parser accepts null values (current buggy behavior)
parser = (
reqparse.RequestParser()
.add_argument("type", type=str, required=True, nullable=True, location="json")
.add_argument("name", type=str, required=True, nullable=True, location="json")
)
with app.test_request_context(json={"type": None, "name": None}, content_type="application/json"):
args = parser.parse_args()
# Step 2: Try to create MetadataArgs with None values
# This should fail at Pydantic validation level
with pytest.raises((ValueError, TypeError)):
metadata_args = MetadataArgs.model_validate(args)
# Step 3: If we bypass Pydantic (simulating the bug scenario)
# Move this outside the request context to avoid Flask-Login issues
mock_metadata_args = Mock()
mock_metadata_args.name = None # From args["name"]
mock_metadata_args.type = None # From args["type"]
mock_user = create_autospec(Account, instance=True)
mock_user.current_tenant_id = "tenant-123"
mock_user.id = "user-456"
with patch(
"services.metadata_service.current_account_with_tenant",
return_value=(mock_user, mock_user.current_tenant_id),
):
# Step 4: Service layer crashes on len(None)
with pytest.raises(TypeError, match="object of type 'NoneType' has no len"):
MetadataService.create_metadata("dataset-123", mock_metadata_args)
def test_correct_nullable_false_configuration_works(self, app):
"""Test that the correct nullable=False configuration works as expected."""
# This tests the FIXED configuration
parser = (
reqparse.RequestParser()
.add_argument("type", type=str, required=True, nullable=False, location="json")
.add_argument("name", type=str, required=True, nullable=False, location="json")
)
with app.test_request_context(json={"type": None, "name": None}, content_type="application/json"):
# This should fail with BadRequest due to nullable=False
from werkzeug.exceptions import BadRequest
with pytest.raises(BadRequest):
parser.parse_args()
valid_payload = {"type": "string", "name": "valid"}
args = MetadataArgs.model_validate(valid_payload)
assert args.type == "string"
assert args.name == "valid"
if __name__ == "__main__":