Fix existence check for tracked + staged ops

This commit is contained in:
Jake Barnby
2025-10-04 19:26:22 +13:00
parent 01ef058adb
commit 3e77810e13
7 changed files with 167 additions and 42 deletions
+8 -3
View File
@@ -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),
@@ -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;
@@ -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)
@@ -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;
@@ -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(...));
}
@@ -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']);
}
}
@@ -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();
}
}