Release v0.1.5 — Phase 05: File Storage & Metadata#76
Merged
Conversation
chore: back-merge main into develop
|
Warning Review limit reached
More reviews will be available in 40 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis pull request introduces a complete file upload system for Laravel that orchestrates file format detection, metadata extraction, storage, and database persistence with comprehensive validation and error handling. ChangesFile Upload System with Format Detection and Storage Orchestration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
database/migrations/2026_05_29_154149_create_files_table.php (1)
15-15: ⚡ Quick winConsider adding a unique constraint to
stored_path.If two
FileRecordinstances accidentally reference the samestored_path, deleting one record (and its associated file on disk) would orphan the other record. While UUID-based path generation makes collisions unlikely, enforcing uniqueness at the database level provides defense-in-depth and prevents data corruption.♻️ Proposed fix to add unique constraint
- $table->string('stored_path'); + $table->string('stored_path')->unique();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/migrations/2026_05_29_154149_create_files_table.php` at line 15, Add a unique constraint on the stored_path column in the CreateFilesTable migration to prevent duplicate file paths: update the up() migration (where $table->string('stored_path') is defined) to create a unique index on stored_path (or make the column unique), and update the down() method to drop that unique index when rolling back so migrations remain reversible; reference the CreateFilesTable class, the up() and down() methods, and the stored_path column when making the change.app/Support/Files/FileExpirationPolicy.php (1)
11-19: 💤 Low valueConsider consolidating duplicate expiration logic.
Both methods currently return identical values. While they accept a
Userparameter for future differentiation (e.g., premium users with longer expiration), the logic could be extracted to a private helper to follow DRY principles.♻️ Optional refactor to reduce duplication
+ private function calculateExpiration(User $user): CarbonInterface + { + return Date::now()->addDay(); + } + public function forUploadedFile(User $user): CarbonInterface { - return Date::now()->addDay(); + return $this->calculateExpiration($user); } public function forResultFile(User $user): CarbonInterface { - return Date::now()->addDay(); + return $this->calculateExpiration($user); }Note: If these methods are expected to diverge soon (e.g., different expiration rules per file type), the current duplication is acceptable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Support/Files/FileExpirationPolicy.php` around lines 11 - 19, Both forUploadedFile(User $user) and forResultFile(User $user) return the same Date::now()->addDay() value; extract that shared logic into a private helper (e.g., private function calculateExpiration(User $user): CarbonInterface) and have both forUploadedFile and forResultFile call this helper, preserving their public signatures so future per-file-type rules (or user-based adjustments) can be added in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Actions/Files/StoreUploadedFileAction.php`:
- Around line 45-46: In StoreUploadedFileAction, validate the filesystem calls
before creating the record: check that $disk->size($path) does not return false
and that $disk->path($path) is valid and hash_file('sha256', $disk->path($path))
does not return false; if either returns false, throw a clear exception or
return an error response (with a descriptive message) instead of passing false
into the record payload so you avoid storing invalid size/checksum or triggering
a DB error.
In `@app/Support/Files/ImageMetadataExtractor.php`:
- Around line 38-41: The method ImageMetadataExtractor::hasTransparency
currently returns $format !== 'jpg' which checks format capability rather than
per-image transparency; rename it to formatSupportsTransparency and update its
signature/usages and tests (tests/Unit/Files/ImageMetadataExtractorTest.php) to
assert capability rather than actual transparency, OR implement real detection
by changing hasTransparency to accept an image path/resource and inspect the
image's alpha channel (for PNG/WebP use imagecreatefromstring + check for any
non-opaque pixels or
imagecolortransparent/imagealphablending/imagecreatetruecolor logic) and update
calls/tests accordingly so the method's name matches its behavior.
---
Nitpick comments:
In `@app/Support/Files/FileExpirationPolicy.php`:
- Around line 11-19: Both forUploadedFile(User $user) and forResultFile(User
$user) return the same Date::now()->addDay() value; extract that shared logic
into a private helper (e.g., private function calculateExpiration(User $user):
CarbonInterface) and have both forUploadedFile and forResultFile call this
helper, preserving their public signatures so future per-file-type rules (or
user-based adjustments) can be added in one place.
In `@database/migrations/2026_05_29_154149_create_files_table.php`:
- Line 15: Add a unique constraint on the stored_path column in the
CreateFilesTable migration to prevent duplicate file paths: update the up()
migration (where $table->string('stored_path') is defined) to create a unique
index on stored_path (or make the column unique), and update the down() method
to drop that unique index when rolling back so migrations remain reversible;
reference the CreateFilesTable class, the up() and down() methods, and the
stored_path column when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fe282805-6bed-408c-b886-3e1f0a3543b9
📒 Files selected for processing (23)
app/Actions/Files/StoreUploadedFileAction.phpapp/Enums/FileStatus.phpapp/Exceptions/Files/FileMetadataException.phpapp/Exceptions/Files/FileStorageException.phpapp/Exceptions/Files/UnsupportedFileFormatException.phpapp/Models/FileRecord.phpapp/Support/Files/FileExpirationPolicy.phpapp/Support/Files/FileFormatDetector.phpapp/Support/Files/FileRecordCreator.phpapp/Support/Files/ImageMetadataExtractor.phpapp/Support/Files/UploadedFileRules.phpdatabase/factories/FileRecordFactory.phpdatabase/migrations/2026_05_29_154149_create_files_table.phptests/Feature/Files/FileExpirationPolicyTest.phptests/Feature/Files/FileOwnershipTest.phptests/Feature/Files/FileRecordTest.phptests/Feature/Files/FileStatusEnumTest.phptests/Feature/Files/FileStorageSmokeTest.phptests/Feature/Files/StoreUploadedFileActionTest.phptests/Feature/Files/StoreUploadedFileCleanupTest.phptests/Unit/Files/FileFormatDetectorTest.phptests/Unit/Files/ImageMetadataExtractorTest.phptests/Unit/Files/UploadedFileRulesTest.php
- StoreUploadedFileAction: validate disk size and hash_file results before persisting record - ImageMetadataExtractor: rename hasTransparency -> formatSupportsTransparency and key supports_transparency to match capability semantics - FileExpirationPolicy: extract shared calculateExpiration() helper - files migration: add unique index on stored_path
This was referenced May 31, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
filestable,FileRecordmodel + factory,FileStatusenum,FileFormatDetector,ImageMetadataExtractor,StoreUploadedFileAction,UploadedFileRules,FileExpirationPolicy,FileRecordCreatorboundary, ownership scope, and cleanup-on-failure.Included tasks
Test plan
composer test— 166 passed, 414 assertionscomposer lint— passednpm run build— passedphp artisan migrate:freshon a clean DBtests/Feature/Filesandtests/Unit/Files🤖 Generated with Claude Code
Summary by cubic
Adds backend support for storing uploaded files and basic metadata using a new
filestable and storage pipeline. Implements format detection, image dimension extraction, validation, ownership, expiration, and safe cleanup; fulfills Linear CONV-058–CONV-070.New Features
filestable andFileRecordmodel withforUserscope; factory included.FileStatusenum and lifecycle fields (status,expires_at).StoreUploadedFileAction: detect format, extract image dimensions, save tolocaldisk, compute size and SHA-256 checksum, create record.UploadedFileRules(png/jpg/jpeg/webp/pdf, max 10 MB).FileExpirationPolicydefaults to 1 day; ownership enforced.Migration
Written for commit c4da252. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests