diff --git a/.jules/bolt.md b/.jules/bolt.md index 57383c3..4b9f670 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -11,3 +11,7 @@ ## 2025-05-15 - [Optimization with Aggregation Pipelines] **Learning:** This codebase frequently uses N+1 query patterns in service methods (e.g., looping through results and calling find_one). These can be significantly optimized using MongoDB aggregation pipelines with $lookup. However, mongomock (used in the test suite) has limited support for advanced $lookup features like 'let' and sub-pipelines. **Action:** Use simple $lookup (localField/foreignField) when possible to maintain test compatibility, and handle any additional filtering or data processing in Python if necessary, which still provides a massive performance win by reducing database roundtrips to 1. + +## 2026-02-25 - [Batch Loading vs. Aggregation Join] +**Learning:** For features that require counts from related collections (like ticket messages), a single `$match` + `$group` aggregation on the target collection followed by Python-side mapping is often cleaner and safer than a complex `$lookup` with sub-pipelines, especially when dealing with mixed-type foreign keys (ObjectId vs. String). +**Action:** Use batch loading with a `$group` aggregation when joining related counts to ensure compatibility and simplicity while still eliminating N+1 bottlenecks. diff --git a/app/services/ticket_service.py b/app/services/ticket_service.py index 7c9224c..7e83c71 100755 --- a/app/services/ticket_service.py +++ b/app/services/ticket_service.py @@ -269,6 +269,43 @@ def get_tickets_by_user(self, username: str, role: str, handlungsfelder: List[st all_tickets = list(all_tickets) logger.debug(f"Alle Tickets gefunden: {len(all_tickets)}") + # Nachrichtenanzahl gesammelt laden (Bolt ⚡ N+1 Fix) + # Nutze Set für automatische Deduplizierung und verbesserte Performance + unique_ticket_ids = set() + for t_list in [open_tickets, assigned_tickets, all_tickets]: + if t_list: + for t in t_list: + if t.get('_id'): + unique_ticket_ids.add(t['_id']) + + # Map für schnellere Zuordnung + message_counts = {} + if unique_ticket_ids: + all_ids_list = list(unique_ticket_ids) + # Eindeutige IDs sammeln (sowohl als ObjectId als auch als String für Kompatibilität) + all_ticket_ids_str = [str(tid) for tid in all_ids_list] + + # Kombiniere beide Formate und dedupliziere für die Abfrage + query_ids = list(set(all_ids_list + all_ticket_ids_str)) + + pipeline = [ + { + '$match': { + 'ticket_id': {'$in': query_ids} + } + }, + { + '$group': { + '_id': '$ticket_id', + 'count': {'$sum': 1} + } + } + ] + results = list(mongodb.aggregate('ticket_messages', pipeline)) + for res in results: + tid = str(res['_id']) + message_counts[tid] = message_counts.get(tid, 0) + res['count'] + # Nachrichtenanzahl und Auftragsdetails hinzufügen logger.debug(f"Verarbeite {len(open_tickets)} offene, {len(assigned_tickets)} zugewiesene, {len(all_tickets)} alle Tickets") @@ -279,15 +316,8 @@ def get_tickets_by_user(self, username: str, role: str, handlungsfelder: List[st # ID-Feld für Template-Kompatibilität ticket['id'] = str(ticket['_id']) - # Nachrichtenanzahl laden (korrekte Collection) - # Unterstütze Messages, deren ticket_id als String oder ObjectId gespeichert ist - messages = mongodb.find('ticket_messages', { - '$or': [ - {'ticket_id': str(ticket['_id'])}, - {'ticket_id': ticket.get('_id')} - ] - }) - ticket['message_count'] = len(list(messages)) + # Nachrichtenanzahl aus der Map zuweisen (Bolt ⚡) + ticket['message_count'] = message_counts.get(ticket['id'], 0) # Auftragsdetails laden (falls vorhanden) if ticket.get('auftrag_details'): diff --git a/tests/unit/test_ticket_service.py b/tests/unit/test_ticket_service.py new file mode 100644 index 0000000..08f1524 --- /dev/null +++ b/tests/unit/test_ticket_service.py @@ -0,0 +1,105 @@ + +import pytest +from bson import ObjectId +from datetime import datetime +from unittest.mock import MagicMock +from app.services.ticket_service import TicketService + +@pytest.fixture +def mock_mongodb(monkeypatch): + # Use a real mongomock client + import mongomock + client = mongomock.MongoClient() + db = client.scandy + + # Mock the mongodb object from app.models.mongodb_database + mock_db = MagicMock() + mock_db.db = db + + def mock_find(collection, query, **kwargs): + return db[collection].find(query) + + def mock_aggregate(collection, pipeline, **kwargs): + return db[collection].aggregate(pipeline) + + def mock_find_one(collection, query, **kwargs): + return db[collection].find_one(query) + + mock_db.find.side_effect = mock_find + mock_db.aggregate.side_effect = mock_aggregate + mock_db.find_one.side_effect = mock_find_one + + monkeypatch.setattr('app.services.ticket_service.mongodb', mock_db) + return db + +def test_get_tickets_by_user_message_count_optimization(mock_mongodb, monkeypatch): + """Verify that message counts are correctly retrieved in batch (Bolt ⚡ optimization)""" + # Setup data + ticket_id = ObjectId() + mock_mongodb.tickets.insert_one({ + '_id': ticket_id, + 'title': 'Test Ticket', + 'status': 'offen', + 'created_at': datetime.now(), + 'updated_at': datetime.now(), + 'deleted': False + }) + + # Add messages with different ID types (both should be counted) + mock_mongodb.ticket_messages.insert_one({ + 'ticket_id': ticket_id, + 'message': 'Message 1' + }) + mock_mongodb.ticket_messages.insert_one({ + 'ticket_id': str(ticket_id), + 'message': 'Message 2' + }) + + # Mock Flask g + mock_g = MagicMock() + mock_g.current_department = None + monkeypatch.setattr('app.services.ticket_service.g', mock_g) + + # Mock ticket_category_service + monkeypatch.setattr('app.services.ticket_service.ticket_category_service', MagicMock()) + + service = TicketService() + + # Call the method (Admin role sees 'all_tickets') + result = service.get_tickets_by_user('testuser', 'admin') + + # Verify message count is correct (2 messages total) + all_tickets = result['all_tickets'] + assert len(all_tickets) == 1 + assert all_tickets[0]['message_count'] == 2 + assert all_tickets[0]['id'] == str(ticket_id) + +def test_get_tickets_by_user_deduplication_and_counts(mock_mongodb, monkeypatch): + """Verify deduplication and message counts for multiple tickets""" + tid1 = ObjectId() + tid2 = ObjectId() + mock_mongodb.tickets.insert_many([ + {'_id': tid1, 'title': 'T1', 'status': 'offen', 'updated_at': datetime.now(), 'deleted': False}, + {'_id': tid2, 'title': 'T2', 'status': 'offen', 'updated_at': datetime.now(), 'deleted': False} + ]) + + # Messages for both tickets + mock_mongodb.ticket_messages.insert_many([ + {'ticket_id': tid1, 'message': 'M1'}, + {'ticket_id': tid1, 'message': 'M2'}, + {'ticket_id': str(tid2), 'message': 'M3'} + ]) + + mock_g = MagicMock() + mock_g.current_department = None + monkeypatch.setattr('app.services.ticket_service.g', mock_g) + + service = TicketService() + monkeypatch.setattr('app.services.ticket_service.ticket_category_service', MagicMock()) + + result = service.get_tickets_by_user('testuser', 'admin') + + # Map results by title for easy assertion + tickets = {t['title']: t for t in result['all_tickets']} + assert tickets['T1']['message_count'] == 2 + assert tickets['T2']['message_count'] == 1