feat: PHP SDK update for version 25.0.0#77
Conversation
Greptile SummaryThis PR updates the Appwrite PHP SDK from v24.2.0 to v25.0.0, adding a new
Confidence Score: 4/5The PR is functionally correct for the happy path, but several optional parameters in the new Organization service and both update methods (Functions and Sites) are assigned to the request body unconditionally, meaning callers who omit them will send explicit nulls to the server. The new Organization service and the updated Functions/Sites services consistently fail to null-guard optional parameters before writing them to $apiParams. In createKey and updateKey the optional expire field is always sent, and in Functions::update() and Sites::update() the new providerBranches and providerPaths fields are written unconditionally while the corresponding create() methods handle them correctly. These issues were flagged in prior review rounds and remain unaddressed. src/Appwrite/Services/Organization.php (createKey/updateKey), src/Appwrite/Services/Functions.php (update), and src/Appwrite/Services/Sites.php (update) Important Files Changed
Reviews (5): Last reviewed commit: "chore: merge main and resolve version co..." | Re-trigger Greptile |
| if (!is_null($providerRootDirectory)) { | ||
| $apiParams['providerRootDirectory'] = $providerRootDirectory; | ||
| } | ||
| $apiParams['providerBranches'] = $providerBranches; | ||
| $apiParams['providerPaths'] = $providerPaths; | ||
|
|
||
| if (!is_null($buildSpecification)) { |
There was a problem hiding this comment.
providerBranches and providerPaths are optional parameters in Sites::update() but are unconditionally added to $apiParams, sending null to the API for any call that omits them. The same pattern (without null checks) also appeared in Functions::update() for the same fields.
| if (!is_null($providerRootDirectory)) { | |
| $apiParams['providerRootDirectory'] = $providerRootDirectory; | |
| } | |
| $apiParams['providerBranches'] = $providerBranches; | |
| $apiParams['providerPaths'] = $providerPaths; | |
| if (!is_null($buildSpecification)) { | |
| if (!is_null($providerRootDirectory)) { | |
| $apiParams['providerRootDirectory'] = $providerRootDirectory; | |
| } | |
| if (!is_null($providerBranches)) { | |
| $apiParams['providerBranches'] = $providerBranches; | |
| } | |
| if (!is_null($providerPaths)) { | |
| $apiParams['providerPaths'] = $providerPaths; | |
| } | |
| if (!is_null($buildSpecification)) { |
| $uploadId = ''; | ||
| $uploadId = $fileId ?? ''; | ||
| $totalChunks = (int) ceil($size / Client::CHUNK_SIZE); |
There was a problem hiding this comment.
The first
$uploadId = ''; assignment is immediately overwritten by $uploadId = $fileId ?? ''; on the next line, making the first line dead code. The indentation inconsistency also suggests a merge/generation artefact.
| $uploadId = ''; | |
| $uploadId = $fileId ?? ''; | |
| $totalChunks = (int) ceil($size / Client::CHUNK_SIZE); | |
| $uploadId = $fileId ?? ''; | |
| $totalChunks = (int) ceil($size / Client::CHUNK_SIZE); |
| $apiParams['keyId'] = $keyId; | ||
| $apiParams['name'] = $name; | ||
| $apiParams['scopes'] = $scopes; | ||
| $apiParams['expire'] = $expire; | ||
|
|
||
| $apiHeaders = []; | ||
| $apiHeaders['content-type'] = 'application/json'; | ||
|
|
||
| $response = $this->client->call( | ||
| Client::METHOD_POST, |
There was a problem hiding this comment.
expire is an optional parameter (?string $expire = null) in createKey but is always written to $apiParams without a null-guard. Every createKey call that omits expire will send null to the API, bypassing any server-side default and potentially causing a validation error.
| $apiParams['keyId'] = $keyId; | |
| $apiParams['name'] = $name; | |
| $apiParams['scopes'] = $scopes; | |
| $apiParams['expire'] = $expire; | |
| $apiHeaders = []; | |
| $apiHeaders['content-type'] = 'application/json'; | |
| $response = $this->client->call( | |
| Client::METHOD_POST, | |
| $apiParams['keyId'] = $keyId; | |
| $apiParams['name'] = $name; | |
| $apiParams['scopes'] = $scopes; | |
| if (!is_null($expire)) { | |
| $apiParams['expire'] = $expire; | |
| } | |
| $apiHeaders = []; | |
| $apiHeaders['content-type'] = 'application/json'; | |
| $response = $this->client->call( | |
| Client::METHOD_POST, |
| $apiParams['keyId'] = $keyId; | ||
| $apiParams['name'] = $name; | ||
| $apiParams['scopes'] = $scopes; | ||
| $apiParams['expire'] = $expire; | ||
|
|
||
| $apiHeaders = []; | ||
| $apiHeaders['content-type'] = 'application/json'; | ||
|
|
||
| $response = $this->client->call( | ||
| Client::METHOD_PUT, |
There was a problem hiding this comment.
expire is optional but assigned to $apiParams unconditionally in updateKey as well, sending null to the API for every call that omits it.
| $apiParams['keyId'] = $keyId; | |
| $apiParams['name'] = $name; | |
| $apiParams['scopes'] = $scopes; | |
| $apiParams['expire'] = $expire; | |
| $apiHeaders = []; | |
| $apiHeaders['content-type'] = 'application/json'; | |
| $response = $this->client->call( | |
| Client::METHOD_PUT, | |
| $apiParams['keyId'] = $keyId; | |
| $apiParams['name'] = $name; | |
| $apiParams['scopes'] = $scopes; | |
| if (!is_null($expire)) { | |
| $apiParams['expire'] = $expire; | |
| } | |
| $apiHeaders = []; | |
| $apiHeaders['content-type'] = 'application/json'; | |
| $response = $this->client->call( | |
| Client::METHOD_PUT, |
This PR contains updates to the PHP SDK for version 25.0.0.