Allow compressed CSV #9673
Allow compressed CSV #9673
Conversation
WalkthroughThe update enhances the CSV migration process to support importing files that may be encrypted and/or compressed. It first detects encryption and compression attributes on the source file. If encrypted, the file is decrypted using OpenSSL with parameters from file attributes and environment variables. If compressed, the content is decompressed using either Zstd or GZIP algorithms. The resulting plaintext CSV is written to the import path. If the file is neither encrypted nor compressed, it is transferred directly. File size is now obtained from the import path. Corresponding tests were updated to reflect the use of a single bucket with encryption and gzip compression enabled and revised test cases accordingly. No changes were made to public or exported entity declarations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Controller
participant OpenSSL
participant Compression_Library
Client->>API_Controller: Upload CSV file (may be encrypted/compressed)
API_Controller->>API_Controller: Check file encryption and compression attributes
alt File is encrypted
API_Controller->>OpenSSL: Decrypt file content using env params
OpenSSL-->>API_Controller: Return decrypted content
alt File is compressed
API_Controller->>Compression_Library: Decompress decrypted content
Compression_Library-->>API_Controller: Return decompressed CSV
else Not compressed
API_Controller->>API_Controller: Use decrypted content as CSV
end
API_Controller->>API_Controller: Write plaintext CSV to import path
API_Controller->>Client: Proceed with import
else File is not encrypted
alt File is compressed
API_Controller->>Compression_Library: Decompress file content
Compression_Library-->>API_Controller: Return decompressed CSV
API_Controller->>API_Controller: Write decompressed CSV to import path
API_Controller->>Client: Proceed with import
else Not compressed
API_Controller->>API_Controller: Transfer file directly to import path
API_Controller->>Client: Proceed with import
end
end
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (20)
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
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/controllers/api/migrations.php (2)
354-356: Consider updating the comment to match PR objectives.The comment suggests compression is skipped on files larger than 20MB, but the PR objective states this update allows compression for files over 20MB. This creates a potential contradiction.
-$hasCompression = $compression !== Compression::NONE; // skipped on files that are 20MB+ in size. +$hasCompression = $compression !== Compression::NONE; // now supporting compression for all file sizes
361-375: Add a default case to handle unsupported compression algorithms.The switch statement doesn't handle cases where the compression algorithm might be something unexpected. Adding a default case would make the code more robust.
switch ($compression) { case Compression::ZSTD: $source = (new Zstd())->decompress($source); break; case Compression::GZIP: $source = (new GZIP())->decompress($source); break; + default: + throw new Exception(Exception::STORAGE_FILE_TYPE_UNSUPPORTED, "Unsupported compression algorithm: {$compression}"); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/controllers/api/migrations.php(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (4)
app/controllers/api/migrations.php (4)
30-31: Appropriate import additions for compression support.These imports enable support for the new compression algorithms (GZIP and Zstd) used in the CSV migration endpoint.
350-352: Improved file restriction logic.The code now only rejects files encrypted with OpenSSL rather than rejecting all compressed files. This change aligns with the PR objective to support compression for larger files.
376-378: Good error handling for file transfer.The error handling for both compressed and uncompressed files is consistent and appropriate.
380-380: Correct file size determination from decompressed file.Now correctly determining the file size from the decompressed file in the import path, which is essential for accurate reporting and processing.
✨ Benchmark results
⚡ Benchmark Comparison
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/controllers/api/migrations.php (1)
352-356: Inline comment is now misleading – please update for clarityThe
// no encryption, compression on files above 20MB.remark reflects the previous behaviour that skipped processing for large files.
Because the surrounding logic now actively supports decrypting / decompressing any size, this sentence will confuse future readers.- // no encryption, compression on files above 20MB. + // Detect whether the file was stored with encryption and/or compression.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/controllers/api/migrations.php(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
What does this PR do?
Files over 20MB are skipped for compressions on storage buckets. This PR allows compressed files now.
Test Plan
Manual.
Related PRs and Issues
Checklist
Summary by CodeRabbit
New Features
Bug Fixes