feat: bump appwrite/appwrite to 23.*#181
Open
premtsd-code wants to merge 9 commits intomainfrom
Open
Conversation
Contributor
Greptile SummaryThis PR upgrades the
Confidence Score: 3/5Not safe to merge yet — unresolved runtime crash paths in Sources/Appwrite.php remain open from prior review rounds, and the PR itself acknowledges further array-offset failures across both adapters. Two cursor-advance patterns in Sources/Appwrite.php dereference a potentially null offset with a property access that will throw a TypeError on an empty page. These were flagged in prior reviews and remain unaddressed. The PR description also explicitly notes that many call sites still use array-offset access on SDK 23 typed responses and will break at runtime. src/Migration/Sources/Appwrite.php — cursor pagination safety on bucket and function page boundaries. Important Files Changed
Reviews (10): Last reviewed commit: "fix(deps): require PHP >=8.2 and fix com..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Adds typed Backups SDK service support (cloud uses this). Fixes PHPStan errors against SDK 23: - Replace removed Runtime::DENO121/124/135 (kept ::DENO140 onward) - Functions::create / Sites::create: specification → buildSpecification + runtimeSpecification (SDK 21 split) - importPasswordUser return type: array → \Appwrite\Models\User|null - API::getRow: call ->toArray() since SDK 22+ returns typed Row NOTE: many other call sites in Sources/Destinations still use array offset access on typed responses. Those paths will break at runtime under SDK 23. This commit is the minimum to ship typed-Backups support in cloud; a follow-up refactor will convert all array-offset access to property access.
SDK 22+ returns typed Models without ArrayAccess, so existing
\$response['key'] patterns fail at runtime ('Cannot use object of
type Appwrite\Models\\\*List as array'). Convert all SDK call sites
in Sources and Destinations to property access:
- \$list['total'] / ['users'|'teams'|...] -> \$list->total / ->users
- \$user['\$id'] -> \$user->id (SDK 22+ drops \$ prefix)
- \$bucket['\$permissions'] -> \$bucket->permissions
API Reader keeps its array contract (matches Database reader, used by
existing callers): converts SDK typed responses via array_map ->toArray
at the I/O boundary.
\Appwrite\Models\Deployment param types added to
exportDeploymentData and exportSiteDeploymentData.
PHPStan level 7 finds zero remaining 'Cannot access offset on
Appwrite\Models\\\*' errors. Level 3 (project default) clean.
…Status) Five runtime bugs missed in the previous regex pass; PHPStan level 8 caught them when checking against the SDK Models. Migration's resource constructors expect plain arrays / strings, not the nested typed objects SDK 23 returns: - \$user->prefs is Preferences object -> use ->data array - \$user->targets is array<Target> -> array_map(fn(\$t) => \$t->toArray()) - \$team->prefs same Preferences fix - \$function->specification doesn't exist on FunctionModel any more; read runtimeSpecification ?: buildSpecification (SDK 21 split) - \$site->specification same Site fix - \$message->status is MessageStatus enum -> cast to string via __toString PHPStan level 3 (project default) clean. Level 8 has no remaining SDK type issues across Sources/Destinations/Reader.
…riable SDK 23.1 added a required \$variableId 2nd parameter: - Sites::createVariable(\$siteId, \$variableId, \$key, \$value, ?\$secret) - Functions::createVariable(\$functionId, \$variableId, \$key, \$value, ?\$secret) Pass the migration resource's id as \$variableId so the destination preserves the source variable id. Bumps appwrite/appwrite to 23.1.0 in composer.lock.
SDK 23.1 inserted \$startCommand between \$buildCommand and \$outputDirectory in Sites::create, which silently shifted positional calls — outputDirectory landed in startCommand and adapter landed in outputDirectory, causing testAppwriteMigrationSite to migrate sites with empty adapter. Fixed by switching to named arguments. Apply the same to: - Functions::create - Functions::createVariable - Sites::createVariable Named args are robust against future positional insertions in the generated SDK.
Migrations from older Appwrite servers may still have functions registered with deno-1.21 / 1.24 / 1.35. SDK 23 dropped those Runtime factory methods. Rather than throwing 'Invalid Runtime' and breaking the migration, transparently bump these deprecated runtimes to deno-1.40, the nearest still-supported version (already mapped).
…140" This reverts commit e5dc657.
end([]) returns false, and PHP's nullsafe operator (?->) only short-circuits on null. end($empty)?->id therefore throws TypeError when the teams response is empty, breaking reportAuth for accounts without teams. Replace with an explicit empty check.
| $currentTables = \array_map(fn ($table) => $table->toArray(), $tablesResponse->tables); | ||
|
|
||
| $tables = \array_merge($tables, $currentTables); | ||
| $lastTable = $tables[count($tables) - 1]['$id'] ?? null; |
Contributor
There was a problem hiding this comment.
toArray() preserving $id key
$lastTable = $tables[count($tables) - 1]['$id'] ?? null (and the subsequent $tableId = $table['$id'] at line 100) depend on ->toArray() emitting the Appwrite dollar-prefixed key '$id'. If SDK 23's toArray() normalises the field to 'id', $lastTable is always null (cursor never advances → potential infinite loop on a full first page), and $tableId becomes null, causing the listRows call to query against a null table ID. This is unchanged context code that now sits downstream of the new ->toArray() conversion — worth confirming the output format explicitly.
appwrite/appwrite 23.x (via SDK 22.0.0) requires PHP >=8.2.0, but this lib's require.php was >=8.1 — composer install would fail on PHP 8.1 consumers. The previous top-level 'platform' key was silently ignored by Composer (must live under 'config.platform'), which hid the inconsistency locally. Move it under 'config.platform' and bump to 8.2 to match the require constraint.
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
Bumps `appwrite/appwrite` to `23.*` so consumers can use the typed `Backups` SDK service shipped in 20.2+.
PHPStan-level fixes:
Known follow-up
Many other call sites in `Sources/Appwrite.php` and `Destinations/Appwrite.php` still use array-offset access on responses (e.g. `$response['rows']`, `$user['$id']`). Those paths will fail at runtime under SDK 23 and need a separate refactor pass to use property access.
Test plan