From 3e77810e13d2ec90b78627860b39cf0af292e6dd Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Sat, 4 Oct 2025 19:26:22 +1300 Subject: [PATCH] Fix existence check for tracked + staged ops --- src/Appwrite/Databases/TransactionState.php | 11 +- .../Http/Databases/Transactions/Create.php | 2 +- .../Transactions/Operations/Create.php | 72 ++++++----- .../Http/Databases/Transactions/Update.php | 3 +- .../Transactions/Operations/Create.php | 1 + .../TablesDB/Transactions/PermissionsBase.php | 113 ++++++++++++++++++ .../Realtime/RealtimeCustomClientTest.php | 7 +- 7 files changed, 167 insertions(+), 42 deletions(-) diff --git a/src/Appwrite/Databases/TransactionState.php b/src/Appwrite/Databases/TransactionState.php index 645c25bf76..2d35ce92ae 100644 --- a/src/Appwrite/Databases/TransactionState.php +++ b/src/Appwrite/Databases/TransactionState.php @@ -7,6 +7,7 @@ use Utopia\Database\Document; use Utopia\Database\Exception; use Utopia\Database\Exception\Timeout; use Utopia\Database\Query; +use Utopia\Database\Validator\Authorization; /** * Service for managing transaction state and providing transaction-aware document operations @@ -351,17 +352,17 @@ class TransactionState */ private function getTransactionState(string $transactionId): array { - $transaction = $this->dbForProject->getDocument('transactions', $transactionId); + $transaction = Authorization::skip(fn () => $this->dbForProject->getDocument('transactions', $transactionId)); if ($transaction->isEmpty() || $transaction->getAttribute('status') !== 'pending') { return []; } // Fetch operations ordered by sequence to replay in exact order - $operations = $this->dbForProject->find('transactionLogs', [ + $operations = Authorization::skip(fn () => $this->dbForProject->find('transactionLogs', [ Query::equal('transactionInternalId', [$transaction->getSequence()]), Query::orderAsc(), Query::limit(PHP_INT_MAX) - ]); + ])); $state = []; @@ -381,6 +382,10 @@ class TransactionState case 'create': $docId = $documentId ?? ($data['$id'] ?? null); if ($docId) { + // Ensure document has the correct $id + if (!isset($data['$id'])) { + $data['$id'] = $docId; + } $state[$collectionId][$docId] = [ 'action' => 'create', 'document' => new Document($data), diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Create.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Create.php index e67c1d08d0..744ad33540 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Create.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Create.php @@ -9,9 +9,9 @@ use Appwrite\SDK\Response as SDKResponse; use Appwrite\Utopia\Response as UtopiaResponse; use Utopia\Database\Database; use Utopia\Database\DateTime; -use Utopia\Database\Validator\Authorization; use Utopia\Database\Document; use Utopia\Database\Helpers\ID; +use Utopia\Database\Validator\Authorization; use Utopia\Swoole\Response as SwooleResponse; use Utopia\Validator\Range; diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Operations/Create.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Operations/Create.php index f7314fb87c..5f6fa4f1a6 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Operations/Create.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Operations/Create.php @@ -3,6 +3,7 @@ namespace Appwrite\Platform\Modules\Databases\Http\Databases\Transactions\Operations; use Appwrite\Auth\Auth; +use Appwrite\Databases\TransactionState; use Appwrite\Extend\Exception; use Appwrite\Platform\Modules\Databases\Http\Databases\Transactions\Action; use Appwrite\SDK\AuthType; @@ -60,11 +61,12 @@ class Create extends Action ->param('operations', [], new ArrayList(new Operation(type: 'legacy')), 'Array of staged operations.', true) ->inject('response') ->inject('dbForProject') + ->inject('transactionState') ->inject('plan') ->callback($this->action(...)); } - public function action(string $transactionId, array $operations, UtopiaResponse $response, Database $dbForProject, array $plan): void + public function action(string $transactionId, array $operations, UtopiaResponse $response, Database $dbForProject, TransactionState $transactionState, array $plan): void { if (empty($operations)) { throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Operations array cannot be empty'); @@ -88,7 +90,7 @@ class Create extends Action $isAPIKey = Auth::isAppUser(Authorization::getRoles()); $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::getRoles()); - $databases = $collections = $staged = []; + $databases = $collections = $staged = $dependants = []; foreach ($operations as $operation) { if (!$isAPIKey && !$isPrivilegedUser && \in_array($operation['action'], [ 'bulkCreate', @@ -122,27 +124,26 @@ class Create extends Action } } - // For update, upsert, delete, check document existence first + // For update, upsert, delete, increment, decrement, check document existence first $document = null; - if (\in_array($operation['action'], ['update', 'delete', 'upsert'])) { + if (\in_array($operation['action'], ['update', 'delete', 'upsert', 'increment', 'decrement'])) { $documentId = $operation[$this->getResourceId()] ?? null; if (empty($documentId)) { throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Document ID is required for ' . $operation['action'] . ' operations'); } - $document = Authorization::skip(fn () => $dbForProject->getDocument( - 'database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), - $documentId - )); + $collectionKey = 'database_' . $database->getSequence() . '_collection_' . $collection->getSequence(); + $isDependant = isset($dependants[$collectionKey][$documentId]); - if ($document->isEmpty() && \in_array($operation['action'], ['update', 'delete'])) { + $document = $transactionState->getDocument($collectionKey, $documentId, $transactionId); + if ($document->isEmpty() && !$isDependant && $operation['action'] !== 'upsert') { throw new Exception(Exception::DOCUMENT_NOT_FOUND); } } $permissionType = match ($operation['action']) { 'create', 'bulkCreate' => Database::PERMISSION_CREATE, - 'update', 'bulkUpdate' => Database::PERMISSION_UPDATE, + 'update', 'bulkUpdate', 'increment', 'decrement' => Database::PERMISSION_UPDATE, 'delete', 'bulkDelete' => Database::PERMISSION_DELETE, 'upsert', 'bulkUpsert' => ($document && !$document->isEmpty()) ? Database::PERMISSION_UPDATE : Database::PERMISSION_CREATE, default => throw new Exception(Exception::GENERAL_BAD_REQUEST, 'Invalid action: ' . $operation['action']) @@ -173,23 +174,21 @@ class Create extends Action // Users can only set permissions for roles they have if (isset($operation['data']['$permissions'])) { - $permissions = $operation['data']['$permissions'] ?? null; - if (!\is_null($permissions)) { - $roles = Authorization::getRoles(); - foreach (Database::PERMISSIONS as $type) { - foreach ($permissions as $permission) { - $permission = Permission::parse($permission); - if ($permission->getPermission() != $type) { - continue; - } - $role = (new Role( - $permission->getRole(), - $permission->getIdentifier(), - $permission->getDimension() - ))->toString(); - if (!Authorization::isRole($role)) { - throw new Exception(Exception::USER_UNAUTHORIZED, 'Permissions must be one of: (' . \implode(', ', $roles) . ')'); - } + $permissions = $operation['data']['$permissions']; + $roles = Authorization::getRoles(); + foreach (Database::PERMISSIONS as $type) { + foreach ($permissions as $permission) { + $permission = Permission::parse($permission); + if ($permission->getPermission() != $type) { + continue; + } + $role = (new Role( + $permission->getRole(), + $permission->getIdentifier(), + $permission->getDimension() + ))->toString(); + if (!Authorization::isRole($role)) { + throw new Exception(Exception::USER_UNAUTHORIZED, 'Permissions must be one of: (' . \implode(', ', $roles) . ')'); } } } @@ -205,17 +204,26 @@ class Create extends Action 'action' => $operation['action'], 'data' => $operation['data'] ?? [], ]); + + // Track create operations for dependent update/increment/decrement/delete operations in same batch + if ($operation['action'] === 'create') { + $collectionKey = 'database_' . $database->getSequence() . '_collection_' . $collection->getSequence(); + $documentId = $operation[$this->getResourceId()] ?? null; + if ($documentId) { + $dependants[$collectionKey][$documentId] = true; + } + } } - $transaction = $dbForProject->withTransaction(function () use ($dbForProject, $transactionId, $staged, $existing, $operations) { - Authorization::skip(fn () => $dbForProject->createDocuments('transactionLogs', $staged)); - return Authorization::skip(fn () => $dbForProject->increaseDocumentAttribute( + $transaction = Authorization::skip(fn () => $dbForProject->withTransaction(function () use ($dbForProject, $transactionId, $staged, $existing, $operations) { + $dbForProject->createDocuments('transactionLogs', $staged); + return $dbForProject->increaseDocumentAttribute( 'transactions', $transactionId, 'operations', \count($operations) - )); - }); + ); + })); $response ->setStatusCode(SwooleResponse::STATUS_CODE_CREATED) diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Update.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Update.php index 0d7ca5c252..d522c699a8 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Update.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Transactions/Update.php @@ -14,16 +14,15 @@ use Appwrite\SDK\Response as SDKResponse; use Appwrite\Utopia\Response as UtopiaResponse; use Utopia\Database\Database; use Utopia\Database\Document; -use Utopia\Database\Exception\Authorization as AuthorizationException; use Utopia\Database\Exception\Conflict as ConflictException; use Utopia\Database\Exception\Duplicate as DuplicateException; -use Utopia\Database\Validator\Authorization; use Utopia\Database\Exception\Limit as LimitException; use Utopia\Database\Exception\NotFound as NotFoundException; use Utopia\Database\Exception\Query as QueryException; use Utopia\Database\Exception\Structure as StructureException; use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Query; +use Utopia\Database\Validator\Authorization; use Utopia\Database\Validator\UID; use Utopia\Swoole\Response as SwooleResponse; use Utopia\Validator\Boolean; diff --git a/src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Operations/Create.php b/src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Operations/Create.php index eab0fed161..d85020b2bc 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Operations/Create.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Operations/Create.php @@ -52,6 +52,7 @@ class Create extends OperationsCreate ->param('operations', [], new ArrayList(new Operation(type: 'tablesdb')), 'Array of staged operations.', true) ->inject('response') ->inject('dbForProject') + ->inject('transactionState') ->inject('plan') ->callback($this->action(...)); } diff --git a/tests/e2e/Services/Databases/TablesDB/Transactions/PermissionsBase.php b/tests/e2e/Services/Databases/TablesDB/Transactions/PermissionsBase.php index 41487b2bb8..9dac59e2ec 100644 --- a/tests/e2e/Services/Databases/TablesDB/Transactions/PermissionsBase.php +++ b/tests/e2e/Services/Databases/TablesDB/Transactions/PermissionsBase.php @@ -667,4 +667,117 @@ trait PermissionsBase // This should fail with 404 Not Found $this->assertEquals(404, $staged['headers']['status-code']); } + + /** + * Test that a document created in one batch can be updated in a subsequent batch within the same transaction + * This validates the transactionState->getDocument() fix for cross-batch dependencies + */ + public function testCanUpdateDocumentCreatedInPreviousBatch(): void + { + $collection = $this->client->call(Client::METHOD_POST, '/tablesdb/' . $this->permissionsDatabase . '/tables', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'tableId' => 'permTest10', + 'name' => 'Permission Test 10', + 'permissions' => [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'rowSecurity' => false, + ]); + + $this->assertEquals(201, $collection['headers']['status-code']); + + $attribute = $this->client->call(Client::METHOD_POST, '/tablesdb/' . $this->permissionsDatabase . '/tables/' . $collection['body']['$id'] . '/columns/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => 'title', + 'size' => 255, + 'required' => true, + ]); + + $this->assertEquals(202, $attribute['headers']['status-code']); + sleep(2); + + // Create transaction + $transaction = $this->client->call(Client::METHOD_POST, '/tablesdb/transactions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(201, $transaction['headers']['status-code']); + + // Batch 1: Create a document + $batch1 = $this->client->call(Client::METHOD_POST, '/tablesdb/transactions/' . $transaction['body']['$id'] . '/operations', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'operations' => [[ + 'action' => 'create', + 'databaseId' => $this->permissionsDatabase, + 'tableId' => $collection['body']['$id'], + 'rowId' => 'crossBatchDoc', + 'data' => [ + 'title' => 'Initial Title', + ], + ]] + ]); + + $this->assertEquals(201, $batch1['headers']['status-code']); + $this->assertEquals(1, $batch1['body']['operations']); + + // Batch 2: Update the document created in batch 1 + $batch2 = $this->client->call(Client::METHOD_POST, '/tablesdb/transactions/' . $transaction['body']['$id'] . '/operations', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'operations' => [[ + 'action' => 'update', + 'databaseId' => $this->permissionsDatabase, + 'tableId' => $collection['body']['$id'], + 'rowId' => 'crossBatchDoc', + 'data' => [ + 'title' => 'Updated Title', + ], + ]] + ]); + + // This should succeed with 201 because transactionState finds the staged document from batch 1 + $this->assertEquals(201, $batch2['headers']['status-code']); + $this->assertEquals(2, $batch2['body']['operations']); + + // Batch 3: Delete the same document + $batch3 = $this->client->call(Client::METHOD_POST, '/tablesdb/transactions/' . $transaction['body']['$id'] . '/operations', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'operations' => [[ + 'action' => 'delete', + 'databaseId' => $this->permissionsDatabase, + 'tableId' => $collection['body']['$id'], + 'rowId' => 'crossBatchDoc', + 'data' => [], + ]] + ]); + + // This should also succeed with 201 + $this->assertEquals(201, $batch3['headers']['status-code']); + $this->assertEquals(3, $batch3['body']['operations']); + + // Rollback to clean up + $rollback = $this->client->call(Client::METHOD_PATCH, '/tablesdb/transactions/' . $transaction['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'rollback' => true, + ]); + + $this->assertEquals(200, $rollback['headers']['status-code']); + } } diff --git a/tests/e2e/Services/Realtime/RealtimeCustomClientTest.php b/tests/e2e/Services/Realtime/RealtimeCustomClientTest.php index 7152925e3a..5a61ae7dc5 100644 --- a/tests/e2e/Services/Realtime/RealtimeCustomClientTest.php +++ b/tests/e2e/Services/Realtime/RealtimeCustomClientTest.php @@ -28,7 +28,7 @@ class RealtimeCustomClientTest extends Scope $userId = $user['$id'] ?? ''; $session = $user['session'] ?? ''; - $headers = [ + $headers = [ 'origin' => 'http://localhost', 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $session ]; @@ -3068,7 +3068,7 @@ class RealtimeCustomClientTest extends Scope 'x-appwrite-project' => $projectId, ], $this->getHeaders()), [ 'documentId' => ID::unique(), - 'data' => [ 'name' => 'L2' ], + 'data' => ['name' => 'L2'], 'permissions' => [ Permission::read(Role::any()), Permission::update(Role::any()), @@ -3082,7 +3082,7 @@ class RealtimeCustomClientTest extends Scope 'x-appwrite-project' => $projectId, ], $this->getHeaders()), [ 'documentId' => ID::unique(), - 'data' => [ 'name' => 'L1' ], + 'data' => ['name' => 'L1'], 'permissions' => [ Permission::read(Role::any()), Permission::update(Role::any()), @@ -3116,5 +3116,4 @@ class RealtimeCustomClientTest extends Scope $client->close(); } - }