fix(storage): add missing encryption and compression properties to File model#11652
fix(storage): add missing encryption and compression properties to File model#11652BillionClaw wants to merge 1 commit intoappwrite:mainfrom
Conversation
…le model The File model was missing the encryption and compression properties that exist in 1.9.x, causing Flutter SDK >= 20.3.3 to crash when accessing file.encryption since it returns null instead of boolean. Added encryption (boolean) and compression (string) fields to the File model schema, and a filter() method that computes these from the actual file attributes (openSSLCipher and algorithm). Fixes appwrite#11647
📝 WalkthroughWalkthroughThis change extends the File response model with two new schema fields: Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Appwrite/Utopia/Response/Model/File.php (1)
117-125: Add PHPDoc for the new publicfilter()override.This new public method should be documented like the other public methods in this class.
As per coding guidelines,**/*.php: Include comprehensive PHPDoc comments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Appwrite/Utopia/Response/Model/File.php` around lines 117 - 125, Add a PHPDoc block above the public function filter(Document $document): Document describing its purpose, parameters, return type, and any thrown exceptions to match the style of other public methods in this class; reference the method name "filter" and the parameter type "Document" so the doc includes an `@param` Document $document, an `@return` Document, a brief description of what attributes are set (compression and encryption) and any relevant tags (e.g., `@see` or `@throws`) consistent with the file's existing PHPDoc conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Appwrite/Utopia/Response/Model/File.php`:
- Around line 88-93: The File model's 'compression' schema currently defaults to
an empty string and other code paths can emit '' which contradicts the
documented closed set (Compression::NONE, Compression::GZIP, Compression::ZSTD);
update the addRule('compression', ...) default from '' to Compression::NONE and
change any emitter/serializer that may return '' (the other occurrence around
the File model emission) to return Compression::NONE when algorithm is missing
so responses always use the documented constants (Compression::NONE/GZIP/ZSTD).
---
Nitpick comments:
In `@src/Appwrite/Utopia/Response/Model/File.php`:
- Around line 117-125: Add a PHPDoc block above the public function
filter(Document $document): Document describing its purpose, parameters, return
type, and any thrown exceptions to match the style of other public methods in
this class; reference the method name "filter" and the parameter type "Document"
so the doc includes an `@param` Document $document, an `@return` Document, a brief
description of what attributes are set (compression and encryption) and any
relevant tags (e.g., `@see` or `@throws`) consistent with the file's existing PHPDoc
conventions.
🪄 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: c7684444-005d-440f-893e-08da3ca00a39
📒 Files selected for processing (1)
src/Appwrite/Utopia/Response/Model/File.php
| ->addRule('compression', [ | ||
| 'type' => self::TYPE_STRING, | ||
| 'description' => 'Compression algorithm used for the file. Will be one of ' . Compression::NONE . ', [' . Compression::GZIP . '](https://en.wikipedia.org/wiki/Gzip), or [' . Compression::ZSTD . '](https://en.wikipedia.org/wiki/Zstd).', | ||
| 'default' => '', | ||
| 'example' => 'gzip' | ||
| ]) |
There was a problem hiding this comment.
Align compression fallback with the documented value set.
Line 90 documents a closed set (none/gzip/zstd), but Line 91 and Line 119 can emit ''. For documents missing algorithm, this can violate the response contract.
Suggested patch
->addRule('compression', [
'type' => self::TYPE_STRING,
'description' => 'Compression algorithm used for the file. Will be one of ' . Compression::NONE . ', [' . Compression::GZIP . '](https://en.wikipedia.org/wiki/Gzip), or [' . Compression::ZSTD . '](https://en.wikipedia.org/wiki/Zstd).',
- 'default' => '',
+ 'default' => 'none',
'example' => 'gzip'
])
@@
public function filter(Document $document): Document
{
- $document->setAttribute('compression', $document->getAttribute('algorithm', ''));
+ $compression = $document->getAttribute('algorithm', 'none');
+ if (!\in_array($compression, ['none', 'gzip', 'zstd'], true)) {
+ $compression = 'none';
+ }
+ $document->setAttribute('compression', $compression);Also applies to: 119-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Appwrite/Utopia/Response/Model/File.php` around lines 88 - 93, The File
model's 'compression' schema currently defaults to an empty string and other
code paths can emit '' which contradicts the documented closed set
(Compression::NONE, Compression::GZIP, Compression::ZSTD); update the
addRule('compression', ...) default from '' to Compression::NONE and change any
emitter/serializer that may return '' (the other occurrence around the File
model emission) to return Compression::NONE when algorithm is missing so
responses always use the documented constants (Compression::NONE/GZIP/ZSTD).
Greptile SummaryThis PR backports the
Confidence Score: 5/5Safe to merge — this is a faithful port of the 1.9.x implementation with no logic changes and no risk of regressions. The change is a direct copy of the well-tested 1.9.x branch code. The filter() pattern is already used by many other models in the same codebase. No data-loss, security, or reliability concerns. The only note is a P2 style suggestion on namespace consistency, which does not affect correctness. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "fix(storage): add missing encryption and..." | Re-trigger Greptile |
|
|
||
| use Appwrite\Utopia\Response; | ||
| use Appwrite\Utopia\Response\Model; | ||
| use Utopia\Compression\Compression; |
There was a problem hiding this comment.
Namespace inconsistency with sibling
Bucket.php
Bucket.php (in the same directory) imports Utopia\Storage\Compression\Compression from the utopia-php/storage package, while this file uses Utopia\Compression\Compression from utopia-php/compression. Both are available as transitive dependencies and likely expose the same constants (NONE, GZIP, ZSTD), so there is no functional bug here — but the inconsistency between two files that describe the same compression concept is worth aligning.
Consider using the same namespace as Bucket.php for consistency across the Response models:
| use Utopia\Compression\Compression; | |
| use Utopia\Storage\Compression\Compression; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Thank you for the review! Added PHPDoc for the new public method as suggested. |
The File model on main was missing the
encryptionandcompressionproperties that exist in the 1.9.x branch. When the Flutter SDK (>= 20.3.3) tries to accessfile.encryption, it returns null instead of a boolean, causing a crash.The fix adds both properties to the File model schema and a
filter()method that derives them from the actual file attributes (openSSLCipherandalgorithm), matching the pattern already present in 1.9.x.Fixes #11647