Add Sites migration support #142
Conversation
WalkthroughAdds first-class Site migration support across the codebase. New resource types Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 215-222: The call to Framework::STATIC() is invalid for Appwrite
v19.1.0 and will fatal; update the Sites creation in Appwrite.php by removing or
replacing Framework::STATIC() in the $this->sites->create(...) call inside the
Resource::TYPE_SITE block: either drop the framework argument if optional, or
map the incoming 'static' value to the correct Framework enum (e.g.,
Framework::REACTSTATIC or another valid Framework constant) before calling
$this->sites->create; update any other occurrences (e.g., the earlier match
handling that uses Framework::STATIC()) to perform the same mapping so the code
uses a valid Framework enum instead of Framework::STATIC().
In `@src/Migration/Transfer.php`:
- Around line 39-43: exportGroupSites currently only triggers exportSites() when
Resource::TYPE_SITE is requested, so requesting Resource::TYPE_SITE_VARIABLE
alone omits variables; update exportGroupSites() to also check for
Resource::TYPE_SITE_VARIABLE and either call exportSites() when variables are
requested or add a separate code path to export variables from exportSites()
output. Modify the conditional that tests for Resource::TYPE_SITE to include
Resource::TYPE_SITE_VARIABLE (or add an explicit branch handling
Resource::TYPE_SITE_VARIABLE) so site variables are exported even if TYPE_SITE
is not present, referencing the GROUP_SITES_RESOURCES constant and using
existing exportSites() logic to perform the actual extraction.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 1542-1635: The code uses Appwrite enums (Framework, Adapter,
BuildRuntime) that may not exist in the installed SDK and will throw fatal
errors; replace enum usage in the match blocks (the mappings that assign
$framework, $adapter, $buildRuntime from
$resource->getFramework()/getAdapter()/getBuildRuntime()) with plain string
values (or a validated string mapping) instead of Framework::*, Adapter::*,
BuildRuntime::* calls, or alternatively add a runtime guard (class_exists for
each enum) and fallback to the raw string when the enum class is missing; update
the match blocks in src/Migration/Destinations/Appwrite.php (the $framework,
$adapter, $buildRuntime assignments) to return strings (e.g., 'react', 'static',
'node-18.0') or use a safe mapping and remove direct enum references unless the
SDK dependency is constrained to a release that includes those enum classes.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 1533-1669: The importSiteResource method may try to create
SiteVariable or SiteDeployment before their parent Site exists because calls
like $resource->getSite()->getId() (used in sites->createVariable) and
importSiteDeployment($resource) assume the Site was already created; fix by
ensuring parent Site exists before handling Resource::TYPE_SITE_VARIABLE and
Resource::TYPE_SITE_DEPLOYMENT — either enforce import ordering upstream or add
a guard in importSiteResource: check that $this->sites has the parent site (or
that $resource->getSite()->getId() resolves to a created site) and if missing,
queue the variable/deployment for retry or throw a clear, recoverable exception
so the caller can import the parent first (apply this check around the
SiteVariable handling block and before calling importSiteDeployment).
- Around line 1675-1732: The Content-Range header in importSiteDeployment is
incorrectly computing the final chunk end as getSize() instead of the inclusive
byte offset getSize() - 1; update the header construction in the client->call
('content-range' value) to use ($deployment->getEnd() == ($deployment->getSize()
- 1) ? $deployment->getSize() - 1 : $deployment->getEnd()) so the final chunk is
"bytes start-(size-1)/size" per RFC 7233; locate this change in
importSiteDeployment where the client->call headers are built (look for
'content-range' and methods getStart(), getEnd(), getSize()).
- Around line 216-223: Replace the empty strings passed to the Appwrite probe so
the SDK validation is exercised: in the block that checks Resource::TYPE_SITE,
update the call to $this->sites->create('', '', Framework::OTHER(),
BuildRuntime::STATIC1()) to use non-empty placeholder values for siteId and name
(e.g. 'test', 'test') so Sites::create() receives realistic parameters and
avoids potential SDK validation errors.
| private function importSiteDeployment(SiteDeployment $deployment): Resource | ||
| { | ||
| $siteId = $deployment->getSite()->getId(); | ||
|
|
||
| if ($deployment->getSize() <= Transfer::STORAGE_MAX_CHUNK_SIZE) { | ||
| $response = $this->client->call( | ||
| 'POST', | ||
| "/sites/{$siteId}/deployments", | ||
| [ | ||
| 'content-type' => 'multipart/form-data', | ||
| ], | ||
| [ | ||
| 'siteId' => $siteId, | ||
| 'code' => new \CURLFile('data://application/gzip;base64,' . base64_encode($deployment->getData()), 'application/gzip', 'deployment.tar.gz'), | ||
| 'activate' => $deployment->getActivated(), | ||
| ] | ||
| ); | ||
|
|
||
| if (!\is_array($response) || !isset($response['$id'])) { | ||
| throw new \Exception('Site deployment creation failed'); | ||
| } | ||
|
|
||
| $deployment->setStatus(Resource::STATUS_SUCCESS); | ||
|
|
||
| return $deployment; | ||
| } | ||
|
|
||
| $response = $this->client->call( | ||
| 'POST', | ||
| "/sites/{$siteId}/deployments", | ||
| [ | ||
| 'content-type' => 'multipart/form-data', | ||
| 'content-range' => 'bytes ' . ($deployment->getStart()) . '-' . ($deployment->getEnd() == ($deployment->getSize() - 1) ? $deployment->getSize() : $deployment->getEnd()) . '/' . $deployment->getSize(), | ||
| 'x-appwrite-id' => $deployment->getId(), | ||
| ], | ||
| [ | ||
| 'siteId' => $siteId, | ||
| 'code' => new \CURLFile('data://application/gzip;base64,' . base64_encode($deployment->getData()), 'application/gzip', 'deployment.tar.gz'), | ||
| 'activate' => $deployment->getActivated(), | ||
| ] | ||
| ); | ||
|
|
||
| if (!\is_array($response) || !isset($response['$id'])) { | ||
| throw new \Exception('Site deployment creation failed'); | ||
| } | ||
|
|
||
| if ($deployment->getStart() === 0) { | ||
| $deployment->setId($response['$id']); | ||
| } | ||
|
|
||
| if ($deployment->getEnd() == ($deployment->getSize() - 1)) { | ||
| $deployment->setStatus(Resource::STATUS_SUCCESS); | ||
| } else { | ||
| $deployment->setStatus(Resource::STATUS_PENDING); | ||
| } | ||
|
|
||
| return $deployment; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Appwrite Sites deployments chunked upload API content-range x-appwrite-id
💡 Result:
For Appwrite Sites deployments (POST /v1/sites/{siteId}/deployments), the code field is a gzip file sent as multipart/form-data. (appwrite.io)
If your deployment archive is > 5MB, Appwrite’s upload endpoints (including Create Deployment) support resumable 5MB chunked uploads using these headers: (appwrite.io)
Content-Range: bytes <first>-<last>/<total>(byte offsets for the current chunk) (appwrite.io)X-Appwrite-ID: <ID>(the upload/deployment/file ID that subsequent chunks belong to) (appwrite.io)
Notes:
- Header names are case-insensitive, so
x-appwrite-idworks the same asX-Appwrite-IDin practice. (appwrite.io) - Pattern is: send the first chunk (server returns an object/ID), then send remaining chunks with the same
Content-Rangescheme andX-Appwrite-IDset to that returned ID. (appwrite.io)
Citations:
- 1: https://appwrite.io/docs/references/cloud/server-rest/sites
- 2: https://appwrite.io/docs/apis/rest
- 3: https://appwrite.io/docs/apis/rest
- 4: https://appwrite.io/docs/apis/rest
- 5: https://appwrite.io/docs/apis/rest
- 6: https://appwrite.io/docs/references/1.2.x/server-rest/storage?utm_source=openai
Fix Content-Range header calculation for final chunk in chunked deployment uploads.
The chunked upload contract (headers and endpoint) aligns with Appwrite's documented API for Sites deployments. However, the Content-Range header has a bug: when constructing the final chunk, the code incorrectly uses $deployment->getSize() as the end byte position instead of $deployment->getSize() - 1.
Per RFC 7233, the end position must be an inclusive 0-based byte offset. For a 1000-byte file, the final chunk should send bytes 800-999/1000, not bytes 800-1000/1000. Change the conditional:
'content-range' => 'bytes ' . ($deployment->getStart()) . '-' . ($deployment->getEnd() == ($deployment->getSize() - 1) ? $deployment->getSize() - 1 : $deployment->getEnd()) . '/' . $deployment->getSize(),🤖 Prompt for AI Agents
In `@src/Migration/Destinations/Appwrite.php` around lines 1675 - 1732, The
Content-Range header in importSiteDeployment is incorrectly computing the final
chunk end as getSize() instead of the inclusive byte offset getSize() - 1;
update the header construction in the client->call ('content-range' value) to
use ($deployment->getEnd() == ($deployment->getSize() - 1) ?
$deployment->getSize() - 1 : $deployment->getEnd()) so the final chunk is "bytes
start-(size-1)/size" per RFC 7233; locate this change in importSiteDeployment
where the client->call headers are built (look for 'content-range' and methods
getStart(), getEnd(), getSize()).
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch the specific server-side code that handles Content-Range parsing
curl -s "https://raw.githubusercontent.com/appwrite/appwrite/f0191202a08d156db43d2b14bd6a0a191ef6f996/src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php" | sed -n '177,186p'
Length of output: 609
@premtsd-code, you're absolutely right that the Appwrite server currently handles this! The server-side code shows it accepts both formats:
if ($end === $fileSize - 1 || $end === $fileSize) {However, I noticed the TODO comment in that same code:
// TODO remove the condition that checks $end === $fileSize in next breaking version
This indicates the server is only accepting $end === $fileSize for backward compatibility and plans to remove it in a future breaking version. The migration code is currently sending the non-RFC-compliant format that will eventually be unsupported.
Recommendation: While it works today, I'd suggest updating the migration code to use the RFC 7233-compliant format ($fileSize - 1) to future-proof against the planned server-side change. This would align with the importDeployment method for functions (lines 1505-1529), which uses the same pattern and would benefit from the same fix.
Would you like to update this proactively, or prefer to wait until the server-side breaking change is announced?
🧬 Code Graph Analysis Results
src/Migration/Resources/Sites/Deployment.php
### File: `src/Migration/Resources/Sites/Deployment.php` ``` class Deployment extends Resource { public function __construct( string $id, private readonly Func $func, private readonly int $size, private int $start = 0, private int $end = 0, private string $data = '', private readonly bool $activated = false ) { $this->id = $id; }/**
* `@param` array<string, mixed> $array
* `@return` self
*/
public static function fromArray(array $array): self
{
return new self(
$array['id'],
Func::fromArray($array['func']),
$array['size'],
$array['start'] ?? 0,
$array['end'] ?? 0,
$array['data'] ?? '',
$array['activated'] ?? false
);
}
/**
* `@return` array<string, mixed>
*/
public function jsonSerialize(): array
{
return [
'id' => $this->id,
'func' => $this->func,
'size' => $this->size,
'start' => $this->start,
'end' => $this->end,
'data' => $this->data,
'activated' => $this->activated,
];
}
public static function getName(): string
{
return Resource::TYPE_DEPLOYMENT;
}
public function getGroup(): string
{
return Transfer::GROUP_FUNCTIONS;
}
public function getFunction(): Func
{
return $this->func;
}
public function getSize(): int
{
return $this->size;
}
public function getEntrypoint(): string
{
return $this->entrypoint;
}
public function setStart(int $start): self
{
$this->start = $start;
return $this;
}
public function getStart(): int
{
return $this->start;
}
public function setEnd(int $end): self
{
$this->end = $end;
return $this;
}
public function getEnd(): int
{
return $this->end;
}
public function setData(string $data): self
{
$this->data = $data;
return $this;
}
public function getData(): string
{
return $this->data;
}
public function getActivated(): bool
{
return $this->activated;
}
}
</snippet>
<filename>src/Migration/Resources/Sites/Deployment.php</filename>
<snippet>
### File: `src/Migration/Resources/Sites/Deployment.php`
class Deployment extends Resource
{
public function __construct(
string $id,
private readonly Func $func,
private readonly int $size,
private int $start = 0,
private int $end = 0,
private string $data = '',
private readonly bool $activated = false
) {
$this->id = $id;
}
/**
* `@param` array<string, mixed> $array
* `@return` self
*/
public static function fromArray(array $array): self
{
return new self(
$array['id'],
Func::fromArray($array['func']),
$array['size'],
$array['start'] ?? 0,
$array['end'] ?? 0,
$array['data'] ?? '',
$array['activated'] ?? false
);
}
/**
* `@return` array<string, mixed>
*/
public function jsonSerialize(): array
{
return [
'id' => $this->id,
'func' => $this->func,
'size' => $this->size,
'start' => $this->start,
'end' => $this->end,
'data' => $this->data,
'activated' => $this->activated,
];
}
public static function getName(): string
{
return Resource::TYPE_SITE_DEPLOYMENT;
}
public function getGroup(): string
{
return Transfer::GROUP_SITES;
}
public function getSite(): Site
{
return $this->site;
}
public function getSize(): int
{
return $this->size;
}
public function setStart(int $start): self
{
$this->start = $start;
return $this;
}
public function getStart(): int
{
return $this->start;
}
public function setEnd(int $end): self
{
$this->end = $end;
return $this;
}
public function getEnd(): int
{
return $this->end;
}
public function setData(string $data): self
{
$this->data = $data;
return $this;
}
public function getData(): string
{
return $this->data;
}
public function getActivated(): bool
{
return $this->activated;
}
}
</snippet>
</details>
---
<details>
<summary>🧠 Learnings used</summary>
Learnt from: premtsd-code
Repo: utopia-php/migration PR: 142
File: src/Migration/Destinations/Appwrite.php:1542-1635
Timestamp: 2026-02-03T15:50:17.680Z
Learning: This repository targets Appwrite PHP SDK v19, and the migration code relies on enum classes Appwrite\Enums\Framework, Appwrite\Enums\BuildRuntime, and Appwrite\Enums\Adapter being present in vendor/appwrite/appwrite/src/Appwrite/Enums. The Sites mappings in src/Migration/Destinations/Appwrite.php intentionally follow the same enum pattern used for Functions.
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
Summary
Changes
Sites/Site.php,Sites/Deployment.php,Sites/EnvVar.phpexportGroupSites,exportSites,exportSiteDeployments,exportSiteDeploymentData,reportSitesinSources/Appwrite.phpimportSiteResource,importSiteDeploymentwith Framework/BuildRuntime/Adapter enum mappings inDestinations/Appwrite.phpTYPE_SITE_DEPLOYMENThandling for cache key resolution and memory leak preventionGROUP_SITESconstant andGROUP_SITES_RESOURCESarrayTYPE_SITE,TYPE_SITE_DEPLOYMENT,TYPE_SITE_VARIABLEconstantsTest plan