cognition upload fix #1674
cognition upload fix #1674
Conversation
WalkthroughThe pull request introduces enhanced validation mechanisms for bot content uploads, focusing on collection limit checks during mass uploading. The changes span multiple files, including the Changes
Sequence DiagramsequenceDiagram
participant User
participant TrainingDataValidator
participant CognitionDataProcessor
User->>TrainingDataValidator: Upload bot content
TrainingDataValidator->>CognitionDataProcessor: Check collection limit
alt Limit Exceeded
CognitionDataProcessor-->>TrainingDataValidator: Return limit error
TrainingDataValidator-->>User: Reject upload with error
else Limit Not Exceeded
TrainingDataValidator->>TrainingDataValidator: Continue validation
TrainingDataValidator-->>User: Allow content upload
end
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
kairon/importer/validator/file_validator.py (1)
1061-1065: Collection limit validation added for mass uploadingThe code adds validation to check if collection limits are exceeded during mass uploading. This is a good addition to prevent resource abuse.
Consider adding error handling for potential exceptions that could occur during the collection limit validation:
+try: if CognitionDataProcessor.is_collection_limit_exceeded_for_mass_uploading(bot, user, new_collection_names, overwrite): bot_content_errors.append('Collection limit exceeded!') +except Exception as e: + logger.error(f"Error validating collection limits: {str(e)}") + bot_content_errors.append(f'Error validating collection limits: {str(e)}')kairon/shared/data/processor.py (1)
Line range hint
4183-4201: Secure file path handling implementedThe code implements secure file path handling with proper validation. This is good for preventing path traversal attacks.
Consider adding these additional security checks:
@staticmethod def get_error_report_file_path(bot: str, event_id: str) -> str: if not event_id.isalnum(): raise HTTPException(status_code=400, detail="Invalid event ID") + if not bot.isalnum(): + raise HTTPException(status_code=400, detail="Invalid bot ID") base_dir = os.path.join('content_upload_summary', bot) + if not os.path.exists(base_dir): + raise HTTPException(status_code=404, detail="Bot directory not found") file_name = f'failed_rows_with_errors_{event_id}.csv' file_path = os.path.join(base_dir, file_name) + # Use realpath to resolve any symlinks + real_path = os.path.realpath(file_path) + real_base = os.path.realpath(base_dir) + if not real_path.startswith(real_base): + raise HTTPException(status_code=400, detail="Invalid file path") if not os.path.exists(file_path) or not file_path.startswith(base_dir): raise HTTPException(status_code=404, detail="Error Report not found") return file_pathtests/unit_test/data_processor/data_processor2_test.py (1)
157-171: Add test cases for edge casesThe test suite should include additional test cases for input validation.
Consider adding these test cases:
@patch('kairon.shared.cognition.processor.MongoProcessor') @patch('kairon.shared.cognition.processor.CognitionSchema') def test_is_collection_limit_exceeded_for_mass_uploading_empty_collections(mock_cognition_schema, mock_mongo_processor): bot = "test_bot" user = "test_user" collection_names = [] with pytest.raises(AppException, match="Collection names cannot be empty!"): CognitionDataProcessor.is_collection_limit_exceeded_for_mass_uploading(bot, user, collection_names) @patch('kairon.shared.cognition.processor.MongoProcessor') @patch('kairon.shared.cognition.processor.CognitionSchema') def test_is_collection_limit_exceeded_for_mass_uploading_empty_bot(mock_cognition_schema, mock_mongo_processor): bot = "" user = "test_user" collection_names = ["collection1"] with pytest.raises(AppException, match="Bot and user are required!"): CognitionDataProcessor.is_collection_limit_exceeded_for_mass_uploading(bot, user, collection_names)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
kairon/importer/validator/file_validator.py(1 hunks)kairon/shared/cognition/processor.py(1 hunks)kairon/shared/data/processor.py(1 hunks)tests/unit_test/data_processor/data_processor2_test.py(2 hunks)tests/unit_test/validator/training_data_validator_test.py(2 hunks)
🔇 Additional comments (2)
kairon/shared/data/processor.py (1)
693-694: Conditional formatting for JSON type data
The code now only formats entries if the type is "json", which is a good optimization to avoid unnecessary processing.
tests/unit_test/validator/training_data_validator_test.py (1)
914-925: LGTM!
The test case properly validates the collection limit exceeded scenario and verifies the correct error message is returned.
fixed download failure for multiple content type
added cognition collection count validation for upload
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores