Add rename_recording endpoint - #117
Conversation
0e91942 to
2c50813
Compare
| raise ValueError(f"Recording not found: participant={participant_name}, filename={old_filename}") | ||
|
|
||
| # Rename directory on disk if it exists | ||
| if os.path.exists(old_filename): |
There was a problem hiding this comment.
Consider revising this to raise an error if the directory does not exist to notify the user and so there is no potential for a DB/filesystem mismatch
There was a problem hiding this comment.
Pull request overview
Adds backend support to rename an existing recording by updating its stored filename and exposing a new FastAPI endpoint for the capture app workflow.
Changes:
- Added
rename_recording_entry()to the SQLite/SQLAlchemy recording DB layer. - Added
POST /rename_recordingendpoint with aRenameRecordingDatarequest model. - Wired the new DB function into the FastAPI backend.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
multi_camera/backend/recording_db.py |
Adds DB-layer rename helper that also attempts to rename on-disk content. |
multi_camera/backend/fastapi.py |
Adds request model + POST /rename_recording route to call the DB-layer helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Rename directory on disk if it exists | ||
| if os.path.exists(old_filename): | ||
| os.rename(old_filename, new_filename) | ||
|
|
||
| recording.filename = new_filename |
There was a problem hiding this comment.
old_filename stored in the DB appears to be a base path prefix (e.g., /.../trial_YYYYMMDD_HHMMSS) while the actual artifacts on disk are *.{serial}.mp4 plus a .json sidecar. os.path.exists(old_filename) will typically be false (no file/dir at that exact path), so the code will skip the filesystem rename but still update the DB, leaving the database and disk out of sync. Consider renaming all expected on-disk artifacts derived from recording.filename (e.g., .json and camera .mp4 files via a glob), and fail the request if none of them exist so you don’t silently desynchronize state.
Adds a POST /api/v1/rename_recording endpoint that renames a recording's directory on disk and updates the filename in the SQLite database. Used by the capture app's debug mode to revise recording filenames after capture.
…recording Validates that new_name is a safe basename (no absolute paths, path separators, or '..' segments) and derives the full path from the existing recording's directory. Checks for DB and on-disk collisions before renaming. Returns HTTP 409 for conflicts.
0c29e66 to
29bc17d
Compare
The capture app sends new_filename as a full relative path, not a bare basename. Revise validation to accept this: reject absolute paths and '..' segments, and verify the parent directory is unchanged (only the basename may differ).
|
Reviewed all four Copilot comments and pushed fixes in two commits (0c29e66, 3853038). 1. Path traversal / input validation (comments 3 and 5) — Fixed. 2. Collision detection (comment 2) — Fixed. 3. Missing directory raises no error (comment 1) — No change. 4. Filename vs directory path mismatch (comment 4) — Not applicable. |
…lot/sub-pr-117 Add rename_recording endpoint with path validation and collision detection
Summary
rename_recording_entry()function to the recording database layer that updates the filename in SQLite and renames the directory on disk.POST /rename_recordingFastAPI endpoint accepting participant, current filename, and new filename.Provides the backend support for IntelligentSensingAndRehabilitation/capture-app#8.