Conversation
… socket_io message
… 403 when user has no access to case
…stead of a string to decode
… 404 when event is not found
… 404 when case is not found
… 400 when the date format is incorrect
… 400 when case_identifier and the event case identifier do not match
… /api/v2/cases/{case_identifier}/events/{identifier}
…when fields are missing
…ith parent_event_id
…cket_io_event_handlers
…r ruff NOQA directive
WalkthroughThis update introduces new socket.io event handlers for cases, notes, and updates by adding dedicated registration functions. The REST endpoints have been refactored to streamline event updating using a consolidated function, with modifications to error handling and logging. Changes have also been made to the socket event registration and notification mechanisms, along with updates to helper functions in the business logic and models for clarity. Additionally, new tests and utilities for managing Socket.IO connections have been added, and outdated socket update handlers have been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST_API
participant BusinessEvents
participant DB
Client->>REST_API: Sends PUT request for event update
REST_API->>BusinessEvents: Invoke events_update()
BusinessEvents->>BusinessEvents: Validate and load event (_load)
BusinessEvents->>DB: Commit updated event data
DB-->>BusinessEvents: Acknowledge update
BusinessEvents-->>REST_API: Return updated event details
REST_API-->>Client: Send response and emit socket notification
sequenceDiagram
participant Client
participant SocketIO
participant HandlerReg
Client->>SocketIO: Emit "join-case-obj-notif" event
SocketIO->>HandlerReg: Invoke socket_join_case_obj_notif()
HandlerReg->>SocketIO: Execute join_room() to add client
SocketIO-->>Client: Confirm room join (acknowledgment)
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
source/app/business/events.py (2)
111-111: Avoid using setattr with constant attribute value.Using
setattrwith a constant attribute value is not any safer than normal property access and adds unnecessary complexity.-setattr(event, 'event_category_id', request_data.get('event_category_id')) +event.event_category_id = request_data.get('event_category_id')🧰 Tools
🪛 Ruff (0.8.2)
111-111: Do not call
setattrwith a constant attribute value. It is not any safer than normal property access.Replace
setattrwith assignment(B010)
128-129: Improve exception chaining in error handling.When catching an exception to raise a new one, you should use
fromto maintain the exception context chain. This helps with debugging by preserving the original traceback.- except marshmallow.exceptions.ValidationError as e: - raise BusinessProcessingError('Data error', data=e.normalized_messages()) + except marshmallow.exceptions.ValidationError as e: + raise BusinessProcessingError('Data error', data=e.normalized_messages()) from e🧰 Tools
🪛 Ruff (0.8.2)
129-129: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
source/app/__init__.py(1 hunks)source/app/blueprints/rest/case/case_timeline_routes.py(4 hunks)source/app/blueprints/rest/v2/auth.py(2 hunks)source/app/blueprints/rest/v2/case_objects/events.py(4 hunks)source/app/blueprints/socket_io_event_handlers/case_event_handlers.py(1 hunks)source/app/blueprints/socket_io_event_handlers/case_notes_event_handlers.py(1 hunks)source/app/blueprints/socket_io_event_handlers/update_event_handlers.py(1 hunks)source/app/business/cases.py(2 hunks)source/app/business/events.py(3 hunks)source/app/iris_engine/updater/updater.py(0 hunks)source/app/iris_engine/utils/collab.py(1 hunks)source/app/models/models.py(2 hunks)tests/iris.py(2 hunks)tests/requirements.txt(1 hunks)tests/socket_io_client.py(1 hunks)tests/socket_io_context_manager.py(2 hunks)tests/tests_rest_events.py(3 hunks)tests/tests_rest_evidences.py(1 hunks)tests/tests_rest_notes.py(1 hunks)
💤 Files with no reviewable changes (1)
- source/app/iris_engine/updater/updater.py
🧰 Additional context used
🧬 Code Graph Analysis (9)
tests/tests_rest_notes.py (2)
tests/iris.py (2)
create_dummy_case(72-80)get_socket_io_client(44-45)tests/socket_io_client.py (2)
emit(32-34)receive(36-39)
source/app/blueprints/socket_io_event_handlers/update_event_handlers.py (1)
source/app/blueprints/socket_io_event_handlers/case_event_handlers.py (1)
get_message(50-54)
source/app/blueprints/rest/case/case_timeline_routes.py (4)
source/app/business/events.py (1)
events_update(97-129)source/app/blueprints/rest/endpoints.py (1)
endpoint_deprecated(76-86)source/app/business/errors.py (2)
BusinessProcessingError(22-32)get_data(31-32)source/app/blueprints/responses.py (1)
response_error(48-54)
tests/socket_io_context_manager.py (1)
tests/socket_io_client.py (3)
SocketIOClient(22-42)connect(29-30)disconnect(41-42)
source/app/__init__.py (3)
source/app/blueprints/socket_io_event_handlers/case_event_handlers.py (1)
register_case_event_handlers(63-68)source/app/blueprints/socket_io_event_handlers/case_notes_event_handlers.py (1)
register_notes_event_handlers(96-105)source/app/blueprints/socket_io_event_handlers/update_event_handlers.py (1)
register_update_event_handlers(44-47)
source/app/business/events.py (6)
source/app/schema/marshables.py (2)
EventSchema(1132-1284)validate_date(1159-1185)source/app/util.py (1)
add_obj_history_entry(72-98)source/app/datamgmt/states.py (1)
update_timeline_state(86-87)source/app/datamgmt/case/case_events_db.py (3)
save_event_category(199-209)update_event_assets(234-277)update_event_iocs(280-306)source/app/business/errors.py (1)
BusinessProcessingError(22-32)source/app/iris_engine/utils/tracker.py (1)
track_activity(30-66)
source/app/blueprints/rest/v2/case_objects/events.py (4)
source/app/business/events.py (2)
events_update(97-129)events_get(90-94)source/app/business/cases.py (1)
cases_exists(71-72)source/app/iris_engine/access_control/utils.py (1)
ac_fast_check_current_user_has_case_access(320-321)source/app/models/authorization.py (1)
CaseAccessLevel(38-45)
source/app/business/cases.py (1)
source/app/models/cases.py (1)
Cases(50-159)
tests/tests_rest_events.py (2)
tests/iris.py (5)
create_dummy_case(72-80)get_socket_io_client(44-45)create(47-48)update(53-54)create_dummy_user(69-70)tests/socket_io_client.py (2)
emit(32-34)receive(36-39)
🪛 Ruff (0.8.2)
source/app/business/events.py
111-111: Do not call setattr with a constant attribute value. It is not any safer than normal property access.
Replace setattr with assignment
(B010)
129-129: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (47)
tests/requirements.txt (1)
2-2: LGTM! Addition of socket.io client supportThe addition of
python-socketio[client]dependency aligns with the PR objective of enhancing socket.io capabilities for testing, particularly focusing on socket.io exchanges.source/app/blueprints/rest/v2/auth.py (2)
28-28: LGTM! Direct import of logger moduleThis change implements the direct import of the
loggermodule, aligning with the PR objective of improving code quality.
47-50: LGTM! Standardized logging implementationThe replacement of
logwith the directly importedloggerconsistently applied to all logging calls enhances code maintainability.tests/iris.py (2)
19-24: LGTM! Socket.IO import additionsThe imports have been properly organized, with the addition of
SocketIOContextManagerto support socket.io testing capabilities.
42-45: LGTM! Socket.IO client integrationGood implementation of the Socket.IO client integration in the Iris test framework. The client is properly initialized with the API URL and API key, and a getter method is provided for access.
tests/tests_rest_evidences.py (1)
266-266: LGTM! Improved code organizationThe test body initialization has been moved closer to where it's used, improving code readability without changing functionality.
tests/tests_rest_notes.py (1)
260-267: Good addition of Socket.IO testThis test verifies the behavior of the socket.io connection for joining the notes overview room. It properly creates a case, connects to socket.io, emits the join event, and validates the response.
The test aligns well with the socket event handling enhancements added in the PR, particularly the 'join-notes-overview' event handler in the
register_notes_event_handlers()function.source/app/__init__.py (1)
174-180: Good organization of Socket.IO event handlersCentralizing the socket.io event handler registration is a clean approach. This restructuring improves code organization by explicitly registering all socket event handlers in one place during application initialization, rather than scattering them throughout the codebase.
The imported registration functions follow a consistent pattern across different domains (cases, notes, updates), which makes the code more maintainable and easier to understand.
source/app/blueprints/socket_io_event_handlers/update_event_handlers.py (4)
26-32: Well-structured message handler for joining update roomsThe implementation follows the same pattern as in other socket handlers, correctly handling room joining and emitting appropriate messages. The function properly extracts the room from the data and uses it in the join_room call.
34-37: Simple and effective ping handlerThis handler provides a straightforward response to ping events, confirming server connectivity which is important for client-side health checks.
39-42: Proper version information handlingThis handler correctly emits the current application version from the app configuration, which is useful for update notifications and version checks.
44-47: Well-organized event handler registrationThis function clearly registers all the update-related socket events with their respective handlers, maintaining a consistent namespace ('/server-updates') which helps avoid conflicts.
source/app/business/cases.py (1)
25-26: Improved type annotations and import organizationAdding the explicit return type annotation for the
_loadfunction enhances code clarity and helps with static type checking. The reordering of imports is also a good practice for maintainability.This change supports the broader functionality of event management in the PR, as the
_loadfunction is used by the newevents_updatefunction.Also applies to: 56-59
source/app/blueprints/rest/case/case_timeline_routes.py (4)
77-77: Good import addition.Explicitly importing the
events_updatefunction enhances code readability and follows the PR objective of using direct imports.
688-688: Great use of deprecation decorator.The
@endpoint_deprecateddecorator clearly indicates to developers that this endpoint is deprecated and provides the new alternative endpoint, which aligns with the PR objective of deprecating older endpoints in favor of the new API v2 structure.
691-699: Good refactoring to simplify the case_edit_event function.Refactoring this function to use the centralized
events_updatefunction reduces code duplication and improves maintainability. The function now has a clearer single responsibility of handling the HTTP request/response while delegating the business logic to the specialized service.
710-711: Improved error handling.Catching the more specific
BusinessProcessingErrorinstead ofmarshmallow.exceptions.ValidationErrorprovides better error handling and more consistent error responses. This change ensures that all business logic errors are properly captured and returned to the client with appropriate messages.tests/socket_io_context_manager.py (2)
19-19: Good import of the SocketIOClient.This import supports the new context manager pattern and properly establishes the dependency relationship.
22-33: Excellent implementation of context manager pattern.The
SocketIOContextManagerclass follows Python's context manager protocol with proper__enter__and__exit__methods. This pattern ensures that socket connections are properly closed even when exceptions occur, preventing resource leaks.The implementation correctly:
- Initializes the client with URL and API key
- Connects on entry and returns the client
- Ensures disconnection on exit
This is a significant improvement over the previous function-based approach and aligns with the PR objective of improving socket.io handling.
source/app/blueprints/socket_io_event_handlers/case_event_handlers.py (2)
57-61: Great addition of notification joining handler.The new
socket_join_case_obj_notiffunction properly handles joining rooms for object notifications with appropriate access control through the@ac_socket_requiresdecorator. This addition helps fix the missing socket.io notification mentioned in the PR objectives.
63-68: Good centralization of event handler registration.The
register_case_event_handlersfunction improves the organization and maintainability of the code by centralizing the registration of all socket.io event handlers in one place. This function ensures that all handlers are properly registered and makes it easier to track which events are being handled.This change aligns with the PR objective of reinstating socket.io event handlers that had been unregistered.
tests/socket_io_client.py (5)
22-28: Well-structured SocketIOClient class initialization.The class is properly initialized with URL and API key parameters, and creates a SimpleClient instance. This design provides a clean interface for socket.io interaction.
29-30: Good connection method with proper authentication.The
connectmethod correctly sets up the connection with authorization headers using the API key, which is crucial for secure socket.io communication.
32-35: Well-implemented emit method with logging.The
emitmethod properly sends events to the specified channel and includes helpful logging to stdout, which will aid in debugging.
36-40: Good receive method with timeout handling.The
receivemethod properly handles message reception with a timeout to prevent indefinite blocking. The logging of received messages is also helpful for debugging.
41-42: Clean disconnect method.The
disconnectmethod ensures proper cleanup by disconnecting the underlying client, which helps prevent resource leaks.source/app/blueprints/socket_io_event_handlers/case_notes_event_handlers.py (1)
96-105: New functionality for centralized socket event registration.The addition of
register_notes_event_handlers()centralizes all event handler registrations in one place, improving code maintainability and making the event registration pattern more explicit.This approach is consistent with modern patterns for socket.io registration and will make it easier to:
- See which events are registered at a glance
- Enable/disable specific handlers programmatically if needed
- Modify registration without changing the handler implementations
source/app/models/models.py (2)
683-687: Simplified conditional logic in Tags.save() method.The refactoring reduces code complexity while maintaining the same functionality. Moving the session operations outside the conditional block makes the flow more straightforward.
985-994: Simplified control flow in create_safe_attr function.The refactoring improves readability by:
- Eliminating the unnecessary else block
- Using early return pattern when an attribute already exists
- Flattening the code structure for better maintainability
This change is aligned with the principle of reducing nesting levels.
source/app/iris_engine/utils/collab.py (2)
3-16: Refactored collab_notify for improved clarity.The change improves the function by:
- Direct import of socket_io, eliminating the app reference
- Creating a structured data object before emitting
- Using proper type hints
The refactoring makes the code more maintainable while preserving the original functionality.
18-26: Added new notification function with simplified interface.The new
notify()function provides a streamlined alternative tocollab_notify()with:
- No request_sid parameter, appropriate for contexts where skipping the sender is not needed
- Direct data object emission without JSON stringification
- Consistent parameter naming with the rest of the API (case_identifier vs case_id)
This addition enhances the notification system while maintaining backward compatibility.
source/app/blueprints/rest/v2/case_objects/events.py (2)
46-58: Function renamed and enhanced with notification.The function has been appropriately renamed from
create_evidencetocreate_eventto better reflect its purpose. The addition of socket.io notification ensures real-time updates for collaborative users.This change improves collaboration by ensuring all users are notified of new events immediately after creation.
85-107: New endpoint for updating events with proper access control.The new
update_eventendpoint follows best practices:
- Proper access control checks
- Validation of case-event relationship
- Error handling for common scenarios (not found, business processing errors)
- Real-time notification of updates
This implementation maintains consistency with other endpoints in the codebase and handles security appropriately.
source/app/business/events.py (3)
21-21: Added import for marshmallow library.The new import is required for the
marshmallow.exceptions.ValidationErrorcatch in theevents_updatefunction. This is a good practice as it makes the specific import clear rather than relying on transitive imports.
42-46: Improved schema validation in the _load function.The schema validation for event date and timezone has been properly implemented. This enhances data integrity by ensuring that dates are validated immediately after loading the event data.
97-127: New events_update function looks good.The new function provides a structured approach for updating events, with proper error handling, history tracking, and database updates. It follows the same pattern as the event creation function, maintaining consistency in the codebase.
🧰 Tools
🪛 Ruff (0.8.2)
111-111: Do not call
setattrwith a constant attribute value. It is not any safer than normal property access.Replace
setattrwith assignment(B010)
tests/tests_rest_events.py (11)
89-103: Good test for Socket.IO notifications on event creation.This test verifies that a Socket.IO message is correctly sent when an event is created. It ensures that event notifications are properly working through the Socket.IO channel, which is important for real-time updates in the UI.
188-200: Proper test for the basic update functionality.This test correctly verifies that the new update endpoint returns a 200 status code on successful update, which is the expected behavior for PUT requests according to REST conventions.
201-213: Good verification of event title updates.This test ensures that the event title is correctly updated in the database after calling the update endpoint, which is a critical aspect of the update functionality.
214-233: Comprehensive Socket.IO test for event updates.This test properly verifies that Socket.IO notifications are sent when events are updated, which ensures that clients are notified of changes in real-time.
234-241: Socket.IO join test looks good.This test verifies that users can properly join a case's notification channel, which is necessary for receiving case-specific notifications.
242-256: Good permission check test.This test correctly verifies that users without proper permissions cannot update events, which is important for security.
257-277: Comprehensive tests for 404 error conditions.These tests verify that appropriate 404 responses are returned when resources don't exist, which is important for proper error handling in the API.
278-290: Good input validation test for date format.This test ensures that the API properly validates date formats and returns a 400 error for invalid formats, which is crucial for data integrity.
291-304: Important validation for case/event relationship.This test verifies that you cannot update an event using a case ID that doesn't match the event's case, which is an important security check.
305-343: Comprehensive tests for required fields.These tests ensure that all required fields are properly validated when updating events, which helps maintain data integrity and prevents incomplete updates.
344-361: Good test for parent-child relationship.This test verifies that the parent-child relationship between events can be properly established during updates, which is important for maintaining event hierarchies.
|
Caution An unexpected error occurred while opening a pull request: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
2 participants
Implement endpoint
PUT /api/v2/cases/{case_identifier}/events/{identifier}to update an evidence.POST /case/timeline/events/update/{event_id}POST /api/v2/cases/{case_identifier}/eventsloggerdirectly instead from theappNote: this PR goes hand in hand with the documentation PR#55 in iris-doc-src.
Summary by CodeRabbit
New Features
Refactor