feat(migration): add project-level resource migrations#186
Open
premtsd-code wants to merge 15 commits into
Open
feat(migration): add project-level resource migrations#186premtsd-code wants to merge 15 commits into
premtsd-code wants to merge 15 commits into
Conversation
Singleton resource per project carrying the 7 auth-method flags (email/password, magic URL, email OTP, anonymous, invites, JWT, phone). Source reads via raw GET /v1/project (no SDK get() method exposed); destination flips each flag via Project::updateAuthMethod(). Renames destination $project (string) -> $projectId so $project can hold the Project SDK service, matching the source-side convention.
Singleton settings resource carrying REST/GraphQL/WebSocket flags. Source reads via raw GET /v1/project; destination flips each via Project::updateProtocol(). Lives in GROUP_SETTINGS.
Singleton settings resource carrying the project's RBAC label array. Source reads via raw GET /v1/project; destination overwrites via Project::updateLabels() (wholesale replace).
Singleton settings resource carrying 17 per-service enable/disable flags (Account, Avatars, Databases, TablesDB, Locale, Health, Project, Storage, Teams, Users, VCS, Sites, Functions, Proxy, GraphQL, Migrations, Messaging). Source reads via raw GET /v1/project; destination flips each via Project::updateService().
The SDK Client's endpoint already includes /v1; calling $this->call('GET',
'/v1/project') produced http://host/v1/v1/project and 404'd. Existing
'/health/version' caller already follows the prefix-less convention.
Affects all five project-singleton sources (AuthMethods, Policies,
Protocols, Labels, Services).
passwordHistory / sessionsLimit / userLimit treat 0 as "disabled" in the source's project document, but the server policy validators reject 0 — they accept a positive int or null. Disabled policies were round-tripping as 0 and the destination rejected the update with: Invalid `total` param: Value must be a valid range between 1 and 5,000 or null Coerce 0 -> null at the SDK call site so the disabled state preserves correctly across the migration.
Replace 9 SDK setter calls (updatePasswordHistoryPolicy, etc.) with a
single dbForPlatform->updateDocument('projects', ...) write. Matches
the convention used by Webhook / Platform / ProjectVariable / ApiKey
destinations — every other project-singleton resource already writes
directly to the project document instead of going through the API.
Drops two server-side workarounds along the way:
- SDK setters return MODEL_PROJECT, which carries cloud's empty
billingLimits as {} that strict-typed SDKs reject ("Missing required
field bandwidth"). Direct DB write never deserializes a Project,
so the bug is irrelevant.
- PasswordHistory / SessionLimit / UserLimit endpoints reject total: 0
even though 0 is the storage convention for "disabled" (see response
model description). Direct DB write writes 0 straight to storage,
bypassing the validator mismatch.
Note: cloud spec fix (appwrite-labs/cloud#4068) and validator fix
(separate appwrite/appwrite PR for Range(0, ...)) are still worth
landing — they help any other SDK consumer hitting the same paths —
but the migration no longer depends on either.
The platform 'projects' collection has document-level permissions restricted to team-owner roles. The migration worker has no team context, so the updateDocument call was being silently rejected by the authorization validator — the migration reported success because no exception bubbled up, but the destination project's auths attribute was left unchanged. Match the upstream Policies/Labels controllers' pattern by wrapping the write in $dbForPlatform->getAuthorization()->skip(...).
- composer.json: appwrite/appwrite ^23 -> ^24 - composer.lock: appwrite/appwrite 23.1.0 -> 24.1.0 - AuthMethod -> ProjectAuthMethodId - ProtocolId -> ProjectProtocolId - ServiceId -> ProjectServiceId 24.1.0 brings the BillingLimits nullable + Project.consoleAccessedAt fix that was blocking the policies migration, and adds Project::get and Project::getPolicy methods used by the source-side refactor.
The /v1/project response doesn't expose per-policy fields at the top level — they live inside the project document's `auths` attribute which the public Project response model omits. Switch from a raw `GET /v1/project` call to per-policy SDK methods (Project::getPolicy(ProjectPolicyId::*)) which return typed policy models (PolicyPasswordHistory, PolicySessionAlert, etc.). Each of the 9 sub-policies maps to one method call.
Contributor
Greptile SummaryThis PR adds end-to-end migration support for five project-level Appwrite resources —
Confidence Score: 5/5The new resource classes and their source/destination integrations are clean and correctly structured; safe to merge. The read-then-merge-then-purge-cache pattern used by all five destination writers operates on strictly disjoint project attributes, so sequential processing produces no stale-data overwrites. The SDK bump is locked to a specific patch and enum renames are applied consistently throughout. No files require special attention beyond what is already noted in the open review threads. Important Files Changed
Reviews (4): Last reviewed commit: "chore: trim verbose comments to match ex..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1650
to
+1708
| code: $e->getCode(), | ||
| previous: $e | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| if (\in_array(Resource::TYPE_WEBHOOK, $resources)) { | ||
| try { | ||
| $this->exportWebhooks($batchSize); | ||
| } catch (\Throwable $e) { | ||
| $this->addError(new Exception( | ||
| Resource::TYPE_WEBHOOK, | ||
| Transfer::GROUP_SETTINGS, | ||
| message: $e->getMessage(), | ||
| code: $e->getCode(), | ||
| previous: $e | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| if (\in_array(Resource::TYPE_PROTOCOLS, $resources)) { | ||
| $this->exportProtocols(); | ||
| } | ||
| } catch (\Throwable $e) { | ||
| $this->addError(new Exception( | ||
| Resource::TYPE_PROTOCOLS, | ||
| Transfer::GROUP_SETTINGS, | ||
| message: $e->getMessage(), | ||
| code: $e->getCode(), | ||
| previous: $e | ||
| )); | ||
| } | ||
|
|
||
| try { | ||
| if (\in_array(Resource::TYPE_LABELS, $resources)) { | ||
| $this->exportLabels(); | ||
| } | ||
| } catch (\Throwable $e) { | ||
| $this->addError(new Exception( | ||
| Resource::TYPE_LABELS, | ||
| Transfer::GROUP_SETTINGS, | ||
| message: $e->getMessage(), | ||
| code: $e->getCode(), | ||
| previous: $e | ||
| )); | ||
| } | ||
|
|
||
| try { | ||
| if (\in_array(Resource::TYPE_SERVICES, $resources)) { | ||
| $this->exportServices(); | ||
| } | ||
| } catch (\Throwable $e) { | ||
| $this->addError(new Exception( | ||
| Resource::TYPE_SERVICES, | ||
| Transfer::GROUP_SETTINGS, | ||
| message: $e->getMessage(), | ||
| code: $e->getCode(), | ||
| previous: $e |
Contributor
There was a problem hiding this comment.
exportGroupSettings
All five addError calls in this method pass code: $e->getCode() directly. Every other export method in this file — including the two new auth handlers above (AUTH_METHODS, POLICIES) at lines 633 and 647 — uses code: (int) $e->getCode() ?: Exception::CODE_INTERNAL. The Exception constructor only remaps a string code that looks non-numeric; an integer 0 (the default for most PHP exceptions) passes through unchanged. This means any unhandled exception from a settings export will produce an error object with code 0 rather than 500, making it invisible to any consumer that filters by HTTP-style status code.
…rvices/Labels Replaces 4 raw 'GET /v1/project' HTTP calls with the typed Project model from SDK 24.1.0. Each resource iterates the typed authMethods/protocols/ services arrays (each containing typed ProjectAuthMethod/ProjectProtocol/ ProjectService models with id+enabled fields) and builds an id->enabled lookup keyed by the corresponding Project*Id enum. Brings the source side fully in line with policies: all 5 settings resources now read via the SDK only, no raw HTTP calls remain.
Comment on lines
+3261
to
+3268
| $existing = $this->dbForPlatform->findOne('webhooks', [ | ||
| Query::equal('projectInternalId', [$this->projectInternalId]), | ||
| Query::equal('name', [$resource->getWebhookName()]), | ||
| ]); | ||
|
|
||
| if ($existing !== false && !$existing->isEmpty()) { | ||
| $resource->setStatus(Resource::STATUS_SKIPPED, 'Webhook already exists'); | ||
| return false; |
Contributor
There was a problem hiding this comment.
Appwrite does not enforce unique webhook names within a project, so a source that legitimately has two webhooks named "Deploy" pointing to different URLs will have the second one silently skipped (STATUS_SKIPPED) on the destination. The migration would complete without error while dropping a live integration.
A safer dedup key is the source $id: after the first successful import, the destination already has that id (or a record keyed on it), so a re-run can skip it without risking a name collision between distinct webhooks.
Unifies the destination side: Protocols, Labels, Services, AuthMethods, and Policies now all write to the project document directly via dbForPlatform->updateDocument(...) wrapped in getAuthorization()->skip(). Each resource bundles its fields into ONE document write instead of N SDK round-trips: - Protocols 3 SDK calls -> 1 document update - Services 17 SDK calls -> 1 document update - AuthMethods 7 SDK calls -> 1 document update - Labels 1 SDK call -> 1 document update (with array_unique dedupe) - Policies (unchanged: was already direct DB) Field mapping mirrors the upstream server handlers: - Protocols -> project.apis (map) - Services -> project.services (map) - Labels -> project.labels (array, deduped) - AuthMethods -> project.auths (map; keys from app/config/auth.php) - Policies -> project.auths (map; shares same attribute as AuthMethods) AuthMethods and Policies both read-then-merge the auths map so they coexist when both ship in the same migration.
Existing private/protected migration functions carry at most a one-line description (most have just @throws). The recent docblocks I added were over-explaining what the code already says. Kept only the two non-obvious WHYs: - createAuthMethods: storage keys differ from SDK enum values; shares the auths map with createPolicies. - createPolicies: SDK setters reject 0 even though 0 = disabled in storage.
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.
Adds support for migrating project-level Appwrite resources end-to-end.
Linear tickets
Resources added
AuthMethodsProject::get()Project::updateAuthMethod()per providerPoliciesProject::getPolicy(ProjectPolicyId::*)× 9dbForPlatform->updateDocument(projects.auths)(bypasses validator that rejects 0=disabled)ProtocolsProject::get()Project::updateProtocol()× 3LabelsProject::get()Project::updateLabels()ServicesProject::get()Project::updateService()× 16Also stacked on this branch
The branch was built on top of three earlier migration features that hadn't been split into their own PRs:
1461c13+ 15 follow-up commits)7fca422)3376bed)These are all in
GROUP_SETTINGSalongside the 5 manager-approved resources and form the complete project-settings migration surface. Splitting them out now would require non-trivial cherry-picking; bundled here for review.SDK dependency bump
appwrite/appwrite:23.*→24.*(locked at 24.1.0)BillingLimitsnullable +Project.consoleAccessedAtno-null fix that was blocking the policies migration (see appwrite-labs/cloud#4068)Project::get()andProject::getPolicy()methods used by the source-side refactorsAuthMethod→ProjectAuthMethodId,ProtocolId→ProjectProtocolId,ServiceId→ProjectServiceId,ProjectPolicy→ProjectPolicyIdNotable design decisions
dbForPlatform->updateDocumentdirectly, not the SDK. The publicProject::updatePolicy*endpoints validate every numeric field as1..5000and reject0, but several policy limits use0 = disabledas their natural off-state. Direct DB write skips that validator.getAuthorization()->skip(...)wraps the call because the migration worker has no team-owner role.Project::getPolicy()× 9, not/v1/project. The/v1/projectresponse model doesn't expose per-policy fields at the top level — they live inside the project document'sauthsattribute, which the public Project response model omits.Files changed
21 files, +1900/-16 lines. Largest diffs are the new resource classes under
src/Migration/Resources/Settings/and thesrc/Migration/Sources/Appwrite.php+src/Migration/Destinations/Appwrite.phpintegrations.