feat: migrate to utopia-php/storage 3.0#12921
Open
loks0n wants to merge 9 commits into
Open
Conversation
Adopt the storage 3.0 major release: - Device factory uses the DeviceType and Acl enums; DSN schemes and the _APP_STORAGE_DEVICE env resolve via DeviceType::tryFrom with unknown values falling back to Local. The DOSpaces setHttpVersion(HTTP/1.1) workaround is gone: the default PSR-18 client already pins HTTP/1.1 - uploadChunk() takes chunk contents instead of a source file path; bounded HTTP chunks are read with file_get_contents and a guard - Storage::human() was removed upstream; replaced by the new Appwrite\Utopia\Bytes::human() helper - The Utopia\Storage\Validator\File SDK-spec placeholder was removed upstream; replaced by Appwrite\Utopia\Request\Validator\File - Health stats unwrap the Telemetry decorator via getDevice() before reading Local-only partition metrics, and report placeholders for non-local devices instead of nonsense -1 byte values - getType() comparisons use DeviceType::Local instead of string constants Blocked on utopia-php/migration supporting storage 3.* before the lock file can resolve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
loks0n
marked this pull request as ready for review
July 20, 2026 13:33
# Conflicts: # composer.lock
Contributor
Greptile SummaryThis PR migrates Appwrite storage integration to utopia-php/storage 3.0. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (7): Last reviewed commit: "feat: singleton storage devices with ten..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+32
to
+35
| public function getType(): string | ||
| { | ||
| return self::TYPE_STRING; | ||
| } |
Contributor
There was a problem hiding this comment.
The upload endpoints now use this replacement validator, but the existing schema mappers only special-case the removed storage validator class for binary multipart fields. When SDK specs are generated for Storage, Functions, or Sites uploads, these parameters can be emitted as plain strings instead of file uploads, so generated clients can send the wrong request shape.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Appwrite/Utopia/Request/Validator/File.php
Line: 32-35
Comment:
**Binary Uploads Become Strings**
The upload endpoints now use this replacement validator, but the existing schema mappers only special-case the removed storage validator class for binary multipart fields. When SDK specs are generated for Storage, Functions, or Sites uploads, these parameters can be emitted as plain strings instead of file uploads, so generated clients can send the wrong request shape.
**Knowledge Base Used:**
- [Functions and Sites: Build and Execution Flow](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/appwrite/appwrite/-/docs/functions-execution.md)
- [Storage, Avatars, and Proxy](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/appwrite/appwrite/-/docs/storage-proxy-avatars.md)
How can I resolve this? If you propose a fix, please make it concise.
✨ Benchmark resultsComparing
Per-scenario breakdown & investigation detailsMetrics below reflect the current branch (after). Δ P95 compares against the base.
Top API waits (after)
|
The GraphQL type mapper and the OpenAPI generator match file parameters by validator class. Both referenced the Utopia\Storage\Validator\File removed in storage 3.0, so file params stopped mapping to the InputFile scalar and multipart binary schema, breaking every GraphQL storage and deployment mutation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # composer.json # composer.lock
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
S3-family devices share a lazy pool of 16 keep-alive cURL connections (getStorageClient() in resources.php): concurrent coroutines borrow a connection exclusively per request, and connections persist between borrows so the TCP/TLS handshake is paid per connection rather than per request. The pool is lazy, so runtimes that never touch S3 dial nothing. Local devices are unaffected. Verified with a Swoole coroutine smoke test against MinIO through a Telemetry-wrapped device: 40 concurrent coroutines, zero errors, 16 connections lazily created at the cap and reused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Devices are immutable per-project value objects (the root is constructor state), so the process-wide singleton lives at the right layer: the connection pool. Fold the static pooled client into getDevice() and drop the separate getStorageClient() helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
getDevice() now memoizes one immutable, coroutine-safe device per (root, connection) pair — roots are the storage base constants, so the memo is bounded — and every S3-family device shares one lazy pool of 16 keep-alive connections. Project scoping moves from constructor state to the new Appwrite\Utopia\Storage\Tenant decorator, which prefixes every path with the tenant segment and rejects any path resolving outside it. This turns per-project isolation from a convention into an enforced invariant: ../ traversal and foreign-tenant paths now throw, closing a gap where S3 paths were never normalized. Project deletion keeps its exact blast radius, since the tenant root is the decorator's root. Verified by unit tests (scoping, traversal rejection, foreign-tenant rejection, root deletion) and a Swoole coroutine smoke test against MinIO through the full singleton + Tenant + Telemetry stack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Adopts the utopia-php/storage 3.0.0 major release: immutable coroutine-safe devices, enums, a PSR-18 transport with S3-aware retries, typed exceptions, and a smaller honest
Devicecontract.Changes
app/init/resources.php): DSN schemes and_APP_STORAGE_DEVICEresolve viaDeviceType::tryFrom()(unknown values fall back toLocal), ACLs use theAclenum, and the DOSpacessetHttpVersion(HTTP/1.1)workaround is deleted — the default PSR-18 client already pins HTTP/1.1.uploadChunk()is data-based upstream; the three call sites (Files, Sites deployments, Functions deployments) read the bounded HTTP chunk withfile_get_contentsplus a failure guard.Storage::human()was removed upstream → newAppwrite\Utopia\Bytes::human()helper (12 call sites).Utopia\Storage\Validator\File(the SDK-spec placeholder) was removed upstream → newAppwrite\Utopia\Request\Validator\File(3 endpoints).Telemetrydecorator viagetDevice()before readingLocal-only partition metrics, and report placeholders for non-local devices — previously S3-backed deployments returned nonsense-1byte values here.getType()comparisons useDeviceType::Localinstead of string constants (Messaging worker, antivirus gate).Singleton devices + tenant scoping
getDevice()memoizes one immutable, coroutine-safe device per (root, connection) pair — roots are the storage base constants, so the memo is bounded — and all S3-family devices share one lazy pool of 16 keep-alive cURL connections (borrowed exclusively per request, persisted between borrows, nothing dialled until first use).Appwrite\Utopia\Storage\Tenantdecorator, which prefixes paths with the tenant segment and rejects any path resolving outside it — turning per-project isolation into an enforced invariant and closing a gap where S3 paths were never../-normalized. Project deletion keeps its exact blast radius (the tenant root is the decorator root).Dependencies
utopia-php/storage2.*→3.0.0utopia-php/migration1.17.*→1.19.0(adds storage 3.0 support, utopia-php/migration#209)Testing
Telemetrydecorator +getDevice(), S3 construction with the default retry client🤖 Generated with Claude Code