- Notifications
You must be signed in to change notification settings - Fork4.6k
Feat txn#10023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:1.8.x
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coderabbitaibot commentedJun 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
📝 WalkthroughWalkthroughThis change introduces a new transaction system for database operations. It adds two new collections ( Suggested reviewers
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat withCodeRabbit:
SupportNeed help? Create a ticket on oursupport page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 13
♻️ Duplicate comments (4)
app/controllers/api/databases.php (4)
4598-4623:Same counter / limit hole forupdateDocumentstaging
updateDocumentwrites a log entry (action =update) but likewise forgets to bumptransactions.operations, leaving the transaction inconsistent and limits unenforced.
5256-5263:Bulk-update staging also forgets to bumpoperationsThe
bulkUpdatepath logs the action but never adjuststransactions.operations, compounding the inconsistency noted above.
5355-5376:Bulk-upsert staging: same missing counter updateIdentical issue – please ensure the transaction document is patched alongside the logs.
5482-5488:Delete-within-transaction not countedThe
deleteDocumentstaging block writes the log but omits the counter update.
🧹 Nitpick comments (7)
app/init/constants.php (1)
55-57:Constants fine, but add unit in the name or php-doc for clarityAll three TTL constants are seconds, while many other timeout constants are suffixed with
_MILLISECONDS.
Consider either renaming (…_SECONDS) or adding a short php-doc to avoid confusion when the values are consumed next to the millisecond ones.src/Appwrite/Utopia/Database/Validator/Operation.php (2)
57-64:Type validation runs before presence validation fordata.Because
dataisn’t guaranteed to exist (see above), theis_array($value['data'])call will still emit a notice even if you add it to$requiredbut leave this block here.
Move thedata-presence check to the loop above or guard witharray_key_exists.
65-69:Inefficientin_arraycall in a hot path.
$this->actionsis static – convert it to aconstarray + useisset($map[$action])for O(1) lookup if performance on large payloads matters.src/Appwrite/Utopia/Response/Model/Transaction.php (2)
31-36:statusfield lacks server-side enforcement of allowed values.Add the
allowedkey to the rule so malformed responses can be caught early.- ->addRule('status', [- 'type' => self::TYPE_STRING,- 'description' => 'Current status of the transaction. One of: pending, committing, committed, rolled_back, failed.',- 'default' => 'pending',- 'example' => 'pending',- ])+ ->addRule('status', [+ 'type' => self::TYPE_STRING,+ 'allowed' => ['pending', 'committing', 'committed', 'rolled_back', 'failed'],+ 'description' => 'Current status of the transaction.',+ 'default' => 'pending',+ 'example' => 'pending',+ ])
12-18:Model is missing theoperationscounter present in the collection.Exposing it would let clients display progress and enforce limits without extra calls.
app/config/collections/projects.php (1)
2515-2567:Consider adding an automatic TTL index onexpiresAt.A regular KEY index won’t clean up rows automatically.
If you rely on MySQL you’ll need a scheduled job; in Mongo-like stores you can switch to a TTL index to avoid manual sweeps.tests/e2e/Services/Databases/DatabasesBase.php (1)
4110-4110:Fix grammatical issue in comment- '$updatedAt' => '2022-08-01 13:09:23.050' // system will update it not api+ '$updatedAt' => '2022-08-01 13:09:23.050' // system will update it, not API
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/config/collections/projects.php(1 hunks)app/config/errors.php(1 hunks)app/config/roles.php(1 hunks)app/config/scopes.php(1 hunks)app/controllers/api/databases.php(31 hunks)app/init/constants.php(1 hunks)src/Appwrite/Extend/Exception.php(1 hunks)src/Appwrite/SDK/Response.php(1 hunks)src/Appwrite/Utopia/Database/Validator/Operation.php(1 hunks)src/Appwrite/Utopia/Database/Validator/Queries/Transactions.php(1 hunks)src/Appwrite/Utopia/Response.php(4 hunks)src/Appwrite/Utopia/Response/Model/Transaction.php(1 hunks)tests/e2e/Services/Databases/DatabasesBase.php(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (16)
src/Appwrite/SDK/Response.php (1)
5-5:readonlyrequires PHP 8.2 – confirm your minimum runtimeDeclaring the whole class as
readonlyis great for immutability, but this syntax is supported only from PHP 8.2 upward.
If the project still targets 8.1 or earlier it will fatal-error at load‐time.
Please double-check thecomposer.json/ CI matrix and bump the platform requirement if necessary.app/config/scopes.php (1)
43-48:Scope names added correctly – remember to wire them into roles & docsNo functional issues spotted with the new
transactions.*scopes.
Just ensure they are:
- Added to the relevant roles in
app/config/roles.php.- Reflected in the public API docs / SDK generation matrix.
app/config/errors.php (1)
813-848:Comprehensive error set – keep codes uniqueThe new transaction error definitions look consistent and follow existing naming & HTTP-code conventions. ✔️
Double-check that no other constant shares the numeric 410/409/400 slots for the same domain to avoid ambiguity in clients.src/Appwrite/Utopia/Database/Validator/Operation.php (1)
80-83:getType()advertises an object whileisArray()returns true.Down-stream consumers might treat TYPE_OBJECT as a map of named properties, not a numerically-indexed array. Verify this aligns with Utopia’s expectations, otherwise return
TYPE_ARRAY.src/Appwrite/Extend/Exception.php (1)
232-240:Constants addition LGTM.Names follow existing style and are unique; no further issues spotted.
app/config/collections/projects.php (1)
2570-2649:Schema and validator drift (actionenum).
transactionLogs.actioncomment listsincrement | decrement, but these are not accepted byOperationvalidator (see earlier comment).
Align both sides to prevent runtime rejects.src/Appwrite/Utopia/Response.php (5)
108-108:Transaction model import added – looks goodThe new
Transactionmodel is correctly imported and used later in the constructor.
169-170:Constants registered for transactions
MODEL_TRANSACTIONandMODEL_TRANSACTION_LISTare consistent with the existing naming scheme and ensure the new entities can be referenced everywhere.
420-420:List registration for transactions
BaseList('Transaction List', … 'transactions', …)wires the collection path and the item model properly. No issues spotted.
519-519:Entity model registration
->setModel(new Transaction())finalises the response-layer support. All good.
375-375: Let’s inspect the surrounding routes to confirm the intended URL segment for Auth Providers:#!/bin/bashsed -n'350,430p' src/Appwrite/Utopia/Response.phpapp/config/roles.php (2)
7-9:Verify that newly added scopes are definedScopes such as
assistant.read,avatars.read,graphql,home, andrules.readhave been added formember roles. Make sure they are present inapp/config/scopes.php; otherwise authentication will reject these tokens.Also applies to: 12-19, 22-30
80-82:Transactions scopes – double-check consistency
transactions.read/transactions.writeare now assigned to admins. Confirm that:
- They are defined in
scopes.php.- All new API routes are protected by the corresponding scope middleware.
If either point is missing, admins might receive 401/403 responses.
tests/e2e/Services/Databases/DatabasesBase.php (3)
5582-5627:Well-structured transaction creation testsThe test properly covers both success and failure scenarios for transaction creation, including TTL validation boundaries.
5778-5860:Comprehensive transaction commit testThe test properly validates the transaction commit flow, including attribute creation wait time and document verification after commit.
3894-3989:Potential data integrity issue in testThe test creates documents with
releaseYearandactorsfields, but these attributes are never defined in the collection schema. Only thetitleattribute is created. This could lead to unexpected behavior or test failures if the system enforces strict schema validation.#!/bin/bash# Check if there are other tests in the codebase that create documents with undefined attributesrg -A 10 -B 5"releaseYear|actors" tests/e2e/Services/Databases/| grep -E"(createAttribute|documents.*data)"| head -20
| public const array ALLOWED_ATTRIBUTES = [ | ||
| 'status', | ||
| 'expiresAt', | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Typed class constant will not parse before PHP 8.3
public const array ALLOWED_ATTRIBUTES … relies on the typed-const RFC that landed in 8.3.
Unless the codebase has already moved to 8.3-minimum this will be a parse error.
- public const array ALLOWED_ATTRIBUTES = [+ /** @var string[] */+ public const ALLOWED_ATTRIBUTES = [
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publicconstarrayALLOWED_ATTRIBUTES = [ | |
| 'status', | |
| 'expiresAt', | |
| ]; | |
| /** @var string[] */ | |
| publicconstALLOWED_ATTRIBUTES = [ | |
| 'status', | |
| 'expiresAt', | |
| ]; |
🤖 Prompt for AI Agents
In src/Appwrite/Utopia/Database/Validator/Queries/Transactions.php around lines7 to 10, the class constant ALLOWED_ATTRIBUTES is declared with a typed array,which is only supported in PHP 8.3 and above. To fix this, remove the typedeclaration "array" from the constant definition so it is compatible withearlier PHP versions. Just declare it as "public const ALLOWED_ATTRIBUTES =[...]" without the type.| public function __construct() | ||
| { | ||
| parent::__construct('functions', self::ALLOWED_ATTRIBUTES); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Wrong collection key passed to parent constructor
parent::__construct('functions', …) should reference thetransactions collection so that query validation works on the right attribute set.
- parent::__construct('functions', self::ALLOWED_ATTRIBUTES);+ parent::__construct('transactions', self::ALLOWED_ATTRIBUTES);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publicfunction __construct() | |
| { | |
| parent::__construct('functions',self::ALLOWED_ATTRIBUTES); | |
| } | |
| publicfunction __construct() | |
| { | |
| parent::__construct('transactions',self::ALLOWED_ATTRIBUTES); | |
| } |
🤖 Prompt for AI Agents
In src/Appwrite/Utopia/Database/Validator/Queries/Transactions.php lines 12 to15, the constructor incorrectly passes 'functions' as the collection key to theparent constructor. Change the first argument from 'functions' to 'transactions'to ensure the query validation uses the correct collection and attribute set.| private array $actions = [ | ||
| 'create', | ||
| 'update', | ||
| 'upsert', | ||
| 'delete', | ||
| 'bulkUpdate', | ||
| 'bulkDelete', | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
🛠️ Refactor suggestion
Allowed-action list is incomplete and already diverges fromtransactionLogs schema.
increment anddecrement appear in the newtransactionLogs collection (size comment at line 2617) but are not accepted here. This validator will therefore reject valid log entries produced by the backend.
Add the missing actions (or, better, keep the list in a shared constant).
- private array $actions = [- 'create',- 'update',- 'upsert',- 'delete',- 'bulkUpdate',- 'bulkDelete',- ];+ private array $actions = [+ 'create',+ 'update',+ 'upsert',+ 'delete',+ 'increment',+ 'decrement',+ 'bulkUpdate',+ 'bulkDelete',+ ];
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private array$actions = [ | |
| 'create', | |
| 'update', | |
| 'upsert', | |
| 'delete', | |
| 'bulkUpdate', | |
| 'bulkDelete', | |
| ]; | |
| private array$actions = [ | |
| 'create', | |
| 'update', | |
| 'upsert', | |
| 'delete', | |
| 'increment', | |
| 'decrement', | |
| 'bulkUpdate', | |
| 'bulkDelete', | |
| ]; |
🤖 Prompt for AI Agents
In src/Appwrite/Utopia/Database/Validator/Operation.php between lines 19 and 26,the $actions array is missing 'increment' and 'decrement' which are validactions according to the transactionLogs schema. To fix this, add 'increment'and 'decrement' to the $actions array. For better maintainability, considerdefining this list as a shared constant used both here and in thetransactionLogs schema to avoid divergence.| foreach ($this->required as $key) { | ||
| if (!\array_key_exists($key, $value)) { | ||
| $this->description = "Missing required key: {$key}"; | ||
| return false; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
data key is accessed later but not declared as required → undefined-index warnings.
You access$value['data'] unconditionally at line 72.
Either add'data' to$required or wrap the later access witharray_key_exists().
- private array $required = [- 'databaseId',- 'collectionId',- 'action',- ];+ private array $required = [+ 'databaseId',+ 'collectionId',+ 'action',+ 'data',+ ];
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($this->requiredas$key) { | |
| if (!\array_key_exists($key,$value)) { | |
| $this->description ="Missing required key:{$key}"; | |
| returnfalse; | |
| } | |
| } | |
| private array$required= [ | |
| 'databaseId', | |
| 'collectionId', | |
| 'action', | |
| 'data', | |
| ]; |
🤖 Prompt for AI Agents
In src/Appwrite/Utopia/Database/Validator/Operation.php around lines 50 to 56,the code checks for required keys but does not include 'data' as a required key,yet $value['data'] is accessed later unconditionally at line 72, causingundefined-index warnings. To fix this, add 'data' to the $required array toensure it is always present or modify the later access to check witharray_key_exists('data', $value) before using $value['data'].| 'releaseYear' => 2000 | ||
| ] | ||
| ]); | ||
| $this->assertEquals($document['body']['title'], 'Creation Date Test'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Undefined attribute in document creation
Similar to the previous test, this creates a document withreleaseYear field that was never defined as an attribute in the collection schema.
🤖 Prompt for AI Agents
In tests/e2e/Services/Databases/DatabasesBase.php around lines 4086 to 4089, thetest creates a document with a 'releaseYear' field that is not defined in thecollection schema attributes. To fix this, update the collection schema toinclude 'releaseYear' as a valid attribute before creating the document, orremove the 'releaseYear' field from the document creation to match the existingschema.| $documents = $this->client->call(Client::METHOD_GET, "/databases/$databaseId/collections/$collectionId/documents", [ | ||
| 'content-type' => 'application/json', | ||
| 'x-appwrite-project' => $this->getProject()['$id'], | ||
| ], $this->getHeaders()); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fix incorrect parameter order in API call
ThegetHeaders() method result should be merged with the headers array, not passed as a separate parameter.
$documents = $this->client->call(Client::METHOD_GET, "/databases/$databaseId/collections/$collectionId/documents", [ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'],- ], $this->getHeaders());+ ] + $this->getHeaders());
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $documents =$this->client->call(Client::METHOD_GET,"/databases/$databaseId/collections/$collectionId/documents", [ | |
| 'content-type' =>'application/json', | |
| 'x-appwrite-project' =>$this->getProject()['$id'], | |
| ],$this->getHeaders()); | |
| $documents =$this->client->call( | |
| Client::METHOD_GET, | |
| "/databases/$databaseId/collections/$collectionId/documents", | |
| [ | |
| 'content-type' =>'application/json', | |
| 'x-appwrite-project' =>$this->getProject()['$id'], | |
| ] +$this->getHeaders() | |
| ); |
🤖 Prompt for AI Agents
In tests/e2e/Services/Databases/DatabasesBase.php around lines 5939 to 5943, thecall to the client's method incorrectly passes the headers array and the resultof getHeaders() as separate parameters. To fix this, merge the array returned bygetHeaders() into the existing headers array so that all headers are combinedinto a single array parameter before passing it to the call method.| $dbForProject->withTransaction(function () use ($dbForProject, $transactionId, $transaction, $requestTimestamp) { | ||
| $dbForProject->updateDocument('transactions', $transactionId, new Document([ | ||
| 'status' => 'committing', | ||
| ])); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Possible fatal error when spreading$operation['data']
$operation['data'] is stored as-is intransactionLogs and can be aDocument instance.
Using the spread operator (...$operation['data']) expects an array and will throw aTypeError for objects.
Coerce to array first:
$data =$operation['data']instanceof Document ?$operation['data']->getArrayCopy() :$operation['data'];...newDocument(['$id' =>$documentId, ...$data]);
🤖 Prompt for AI Agents
In app/controllers/api/databases.php around lines 1772 to 1776, the code usesthe spread operator on $operation['data'], which can be a Document object,causing a TypeError. To fix this, check if $operation['data'] is an instance ofDocument and convert it to an array using getArrayCopy() before spreading it.Replace the spread usage with a variable $data that holds the coerced array,then spread $data inside the new Document constructor.| $dbForProject->withTransaction(function () use ($dbForProject, $transactionId, $staged, $existing, $operations) { | ||
| $dbForProject->createDocuments('transactionLogs', $staged); | ||
| $dbForProject->updateDocument('transactions', $transactionId, new Document([ | ||
| 'operations' => $existing + \count($operations), | ||
| ])); | ||
| }); | ||
| $response | ||
| ->setStatusCode(Response::STATUS_CODE_CREATED) | ||
| ->dynamic($transaction, Response::MODEL_TRANSACTION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
🛠️ Refactor suggestion
Response shows staleoperations counter
createOperations updates the counter inside the DB, but the$transaction object returned to the client is the pre-update instance – it still shows the oldoperations value.
Fetch the updated document (or mutate the local$transaction) before calling$response->dynamic(...).
🤖 Prompt for AI Agents
In app/controllers/api/databases.php around lines 1639 to 1648, the responsereturns the $transaction object before the operations counter is updated in thedatabase, causing stale data to be sent to the client. To fix this, after thetransaction update inside the withTransaction callback, fetch the updatedtransaction document from the database or update the local $transaction variableto reflect the new operations count before passing it to$response->dynamic(...).| switch ($operation['action']) { | ||
| case 'create': | ||
| $creates[$databaseInternalId][$collectionInternalId][] = new Document([ | ||
| '$id' => $documentId ?? ID::unique(), | ||
| ...$operation['data'] | ||
| ]); | ||
| break; | ||
| case 'update': | ||
| case 'upsert': | ||
| $updates[$databaseInternalId][$collectionInternalId][] = new Document([ | ||
| '$id' => $documentId, | ||
| ...$operation['data'], | ||
| ]); | ||
| break; | ||
| case 'delete': | ||
| $deletes[$databaseInternalId][$collectionInternalId][] = $documentId; | ||
| break; | ||
| case 'bulkUpdate': | ||
| $bulkUpdates[$databaseInternalId][$collectionInternalId][] = [ | ||
| 'data' => $operation['data'] ?? null, | ||
| 'queries' => $operation['queries'] ?? [], | ||
| ]; | ||
| break; | ||
| case 'bulkDelete': | ||
| $bulkDeletes[$databaseInternalId][$collectionInternalId][] = [ | ||
| 'queries' => $operation['queries'] ?? [], | ||
| ]; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
increment /decrement actions are silently dropped on commit
updateTransaction’s commit loop never handles theincrement anddecrement actions that are being written totransactionLogs by the respective endpoints.
As a result, those staged mutations are lost – the transaction is markedcommitted while the underlying data never changes.
Add explicit branches (or reuseincreaseDocumentAttribute /decreaseDocumentAttribute) before setting the transaction status tocommitted.
🤖 Prompt for AI Agents
In app/controllers/api/databases.php around lines 1788 to 1816, the switchstatement handling transaction commit actions does not process 'increment' and'decrement' actions, causing those mutations to be lost. To fix this, addexplicit cases for 'increment' and 'decrement' in the switch, and call theappropriate functions like increaseDocumentAttribute anddecreaseDocumentAttribute to apply these changes before marking the transactionas committed.| if (!empty($transactionId)) { | ||
| $operations = []; | ||
| foreach ($documents as $document) { | ||
| $operations[] = new Document([ | ||
| 'databaseInternalId' => $database->getSequence(), | ||
| 'collectionInternalId' => $collection->getSequence(), | ||
| 'transactionInternalId' => $transaction->getSequence(), | ||
| 'documentId' => $document->getId(), | ||
| 'action' => 'create', | ||
| 'data' => $document->getArrayCopy(), | ||
| ]); | ||
| } | ||
| $queueForEvents | ||
| ->setParam('databaseId', $databaseId) | ||
| ->setParam('collectionId', $collection->getId()) | ||
| ->setContext('collection', $collection) | ||
| ->setContext('database', $database); | ||
| try { | ||
| $dbForProject->createDocuments('transactionLogs', $operations); | ||
| } catch (DuplicateException) { | ||
| throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); | ||
| } catch (NotFoundException) { | ||
| throw new Exception(Exception::COLLECTION_NOT_FOUND); | ||
| } catch (RelationshipException $e) { | ||
| throw new Exception(Exception::RELATIONSHIP_VALUE_INVALID, $e->getMessage()); | ||
| } catch (StructureException $e) { | ||
| throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $e->getMessage()); | ||
| } | ||
| } else { | ||
| try { | ||
| $dbForProject->createDocuments( | ||
| 'database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), | ||
| $documents | ||
| ); | ||
| } catch (DuplicateException) { | ||
| throw new Exception(Exception::DOCUMENT_ALREADY_EXISTS); | ||
| } catch (NotFoundException) { | ||
| throw new Exception(Exception::COLLECTION_NOT_FOUND); | ||
| } catch (RelationshipException $e) { | ||
| throw new Exception(Exception::RELATIONSHIP_VALUE_INVALID, $e->getMessage()); | ||
| } catch (StructureException $e) { | ||
| throw new Exception(Exception::DOCUMENT_INVALID_STRUCTURE, $e->getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
transactions.operations never updated for staged single-document creates
WhentransactionId is supplied tocreateDocument, an entry is pushed totransactionLogs but the parent transaction’soperations counter isnot incremented nor is the batch-limit checked.
This bypasses the safety limits enforced in/operations, making it possible to exceeddatabasesBatchSize.
Update the transaction inside the same DB transaction used for logging:
$dbForProject->withTransaction(function () use ($dbForProject, $transaction) { $dbForProject->createDocuments('transactionLogs', $operations);+ $dbForProject->updateDocument('transactions', $transaction->getId(), new Document([+ 'operations' => $transaction->getAttribute('operations') + count($operations),+ ]));});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/controllers/api/databases.php around lines 3896 to 3935, whentransactionId is provided, the code logs operations to transactionLogs but doesnot update the parent transaction's operations counter or check the batch limit,which can bypass safety limits. To fix this, modify the code to increment thetransaction's operations count and perform the batch size check within the samedatabase transaction that logs the operations, ensuring consistency andenforcing limits properly.
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist