Skip to content

Add Custom Domains migration#188

Open
premtsd-code wants to merge 3 commits into
add-smtp-migrationfrom
add-custom-domains-migration
Open

Add Custom Domains migration#188
premtsd-code wants to merge 3 commits into
add-smtp-migrationfrom
add-custom-domains-migration

Conversation

@premtsd-code
Copy link
Copy Markdown
Contributor

Summary

  • Adds a new GROUP_DOMAINS resource group with Resource::TYPE_RULE for migrating custom-domain proxy rules.
  • Source (Sources/Appwrite) lists rules via the console SDK's Proxy service, paginated with cursor.
  • Destination (Destinations/Appwrite) dispatches to the SDK's four proxy-rule create endpoints (createAPIRule, createFunctionRule, createSiteRule, createRedirectRule) based on the rule's type + deploymentResourceType.
  • Auto-generated .appwrite.network rules are skipped — they're recreated automatically when the parent Function/Site is migrated.
  • Cross-project domain uniqueness conflicts (409) are surfaced as STATUS_WARNING rather than aborting the migration, since the source must release the domain first.
  • Stubbed exportGroupDomains in Firebase / NHost / JSON / CSV sources to satisfy the new abstract method.

Test plan

  • CI lints + tests green
  • E2E testAppwriteMigrationCustomDomains (in appwrite/appwrite) passes

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR adds custom-domain proxy rule migration support: a new Rule resource, GROUP_DOMAINS group, source export (Appwrite SDK's Proxy service with cursor pagination), and destination import dispatching to the four createAPIRule / createFunctionRule / createSiteRule / createRedirectRule endpoints. Auto-generated .appwrite.network rules are skipped, and 409 conflicts surface as warnings rather than aborting the migration.

  • New Rule resource & GROUP_DOMAINS groupResources/Domains/Rule.php models all proxy-rule fields; Transfer::GROUP_DOMAINS_RESOURCES and the corresponding constants are wired through Source, Transfer, and both Appwrite adapters.
  • Source exportexportRules() paginates via cursor, maps SDK response objects to Rule instances, and wraps errors with typed Exception objects.
  • Destination importcreateRule() skips auto-generated rules, dispatches on type + deploymentResourceType, and surfaces domain-ownership conflicts (409) as STATUS_WARNING; all other sources stub exportGroupDomains() as not implemented.

Confidence Score: 4/5

Mostly safe, but Transfer.php still has a gap where GROUP_SETTINGS cannot be expanded via extractServices(), which will throw for any caller using that group.

The domains migration logic itself is well-structured, but GROUP_SETTINGS remains absent from extractServices() even though this PR correctly added GROUP_DOMAINS. Since GROUP_SETTINGS_RESOURCES is now fully populated, any caller passing GROUP_SETTINGS to extractServices() hits the default throw branch — a real defect on the changed path in Transfer.php.

src/Migration/Transfer.php — the extractServices() match expression is missing GROUP_SETTINGS.

Important Files Changed

Filename Overview
src/Migration/Transfer.php Adds GROUP_DOMAINS constant and GROUP_DOMAINS_RESOURCES array, and wires GROUP_DOMAINS into extractServices() — but GROUP_SETTINGS is still absent from extractServices(), causing a thrown exception for any caller using that group.
src/Migration/Destinations/Appwrite.php Adds importDomainsResource() and createRule() dispatching to four proxy-rule endpoints; redirect rule case has a silent fallback to FUNCTION type when deploymentResourceType is empty.
src/Migration/Sources/Appwrite.php Adds exportGroupDomains() and exportRules() with cursor-based pagination; minor edge case where callback is invoked with an empty array when rule count is an exact multiple of batchSize.
src/Migration/Resources/Domains/Rule.php New Rule resource class with full constructor, fromArray(), jsonSerialize(), getName(), and getGroup() — looks correct and complete.
src/Migration/Source.php Adds getDomainsBatchSize(), GROUP_DOMAINS mapping in the resource loop, and the abstract exportGroupDomains() method — straightforward, consistent with existing patterns.
src/Migration/Resource.php Adds TYPE_RULE constant and includes it in the all-resources list — minimal, correct change.
src/Migration/Sources/CSV.php Stubbed exportGroupDomains() throwing 'Not Implemented' — satisfies the new abstract method contract.
src/Migration/Sources/Firebase.php Stubbed exportGroupDomains() throwing 'Not implemented' — satisfies the new abstract method contract.
src/Migration/Sources/JSON.php Stubbed exportGroupDomains() throwing 'Not Implemented' — satisfies the new abstract method contract.
src/Migration/Sources/NHost.php Stubbed exportGroupDomains() throwing 'Not Implemented' — satisfies the new abstract method contract.
tests/Migration/Unit/Adapters/MockSource.php Implements exportGroupDomains() in MockSource consistent with the existing mock pattern for other groups.

Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/add..." | Re-trigger Greptile

Comment on lines 449 to 451
self::GROUP_BACKUPS => array_merge($resources, self::GROUP_BACKUPS_RESOURCES),
self::GROUP_DOMAINS => array_merge($resources, self::GROUP_DOMAINS_RESOURCES),
default => throw new \Exception('No service group found'),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 GROUP_SETTINGS still missing from extractServices()

GROUP_DOMAINS was added to the extractServices() match expression in this PR, but GROUP_SETTINGS is still absent. Any caller that passes Transfer::GROUP_SETTINGS to extractServices() will hit the default branch and throw \Exception('No service group found'), making the entire settings group unexpandable through this method even though GROUP_SETTINGS_RESOURCES is fully populated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant