diff --git a/dev/test-distributed-lock.sh b/dev/test-distributed-lock.sh deleted file mode 100755 index 6a740a4169..0000000000 --- a/dev/test-distributed-lock.sh +++ /dev/null @@ -1,157 +0,0 @@ -#!/usr/bin/env bash -# -# Manual smoke test for the distributed-lock pilot on `updateProjectService`. -# -# Fires N concurrent PATCH /project/services/:serviceId requests against the -# same project, each toggling a different service to "enabled=true". Then -# refetches the project and counts how many of the targeted services actually -# persisted. -# -# Usage: -# APPWRITE_ENDPOINT=http://localhost \ -# APPWRITE_PROJECT_ID= \ -# APPWRITE_API_KEY= \ -# ./dev/test-distributed-lock.sh -# -# Required scopes on the API key: -# - project.write (to toggle services) -# - projects.read (to refetch project state via GET /v1/projects/:id) -# -# To prove the lock fixes the bug, run twice: -# 1. With `_APP_LOCKING_ENABLED=enabled` (default): expect successes == enabled -# 2. With `_APP_LOCKING_ENABLED=disabled` : expect successes > enabled (lost updates) -# -# Set `_APP_LOCKING_ENABLED` in `.env` and `docker compose up -d --force-recreate` -# between runs. Use `--parallelism N` to tune the concurrency level. - -set -eu - -# --- Configuration --------------------------------------------------------- - -ENDPOINT="${APPWRITE_ENDPOINT:?set APPWRITE_ENDPOINT, e.g. http://localhost}" -PROJECT_ID="${APPWRITE_PROJECT_ID:?set APPWRITE_PROJECT_ID}" -API_KEY="${APPWRITE_API_KEY:?set APPWRITE_API_KEY}" -PARALLELISM="${PARALLELISM:-5}" - -# Services to toggle concurrently — must be in the optional-services list of -# the project. These are the same set used by the e2e ServicesBase trait. -SERVICES=(teams storage functions sites messaging) - -# Trim or extend SERVICES to match PARALLELISM. -SERVICES=("${SERVICES[@]:0:$PARALLELISM}") - -if [ "${#SERVICES[@]}" -lt 2 ]; then - echo "ERROR: PARALLELISM must be >= 2 to detect contention" >&2 - exit 1 -fi - -# --- Helpers --------------------------------------------------------------- - -curl_appwrite() { - local method="$1" - local path="$2" - shift 2 - curl -sS -o /tmp/lock-smoke-body.$$ -w '%{http_code}' \ - -X "$method" \ - -H "Content-Type: application/json" \ - -H "X-Appwrite-Project: $PROJECT_ID" \ - -H "X-Appwrite-Key: $API_KEY" \ - "$ENDPOINT/v1$path" \ - "$@" -} - -toggle_service() { - local service="$1" - local enabled="$2" - local code - code=$(curl_appwrite PATCH "/project/services/$service" \ - -d "{\"enabled\": $enabled}") - echo "$code" -} - -get_project_state() { - curl -sS \ - -H "Content-Type: application/json" \ - -H "X-Appwrite-Project: console" \ - -H "X-Appwrite-Key: $API_KEY" \ - "$ENDPOINT/v1/projects/$PROJECT_ID" -} - -# --- Run ------------------------------------------------------------------- - -echo "==> Distributed-lock smoke test" -echo " endpoint: $ENDPOINT" -echo " project: $PROJECT_ID" -echo " parallelism: $PARALLELISM" -echo " services: ${SERVICES[*]}" -echo - -# 1. Baseline — disable all targeted services sequentially. -echo "==> Baseline: disabling ${SERVICES[*]}" -for svc in "${SERVICES[@]}"; do - code=$(toggle_service "$svc" false) - if [ "$code" != "200" ]; then - echo " WARN: baseline disable of $svc returned $code (expected 200)" - fi -done - -# 2. Fire concurrent toggles to enabled=true. Capture each child's HTTP status. -echo -echo "==> Firing ${#SERVICES[@]} concurrent toggle requests..." -RESULTS_FILE=$(mktemp -t lock-smoke-results.XXXXXX) -for svc in "${SERVICES[@]}"; do - ( - code=$(toggle_service "$svc" true) - printf '%s %s\n' "$svc" "$code" >> "$RESULTS_FILE" - ) & -done -wait - -# 3. Tally responses. -SUCCESS_COUNT=$(awk '$2 == 200' "$RESULTS_FILE" | wc -l | tr -d ' ') -CONFLICT_COUNT=$(awk '$2 == 409' "$RESULTS_FILE" | wc -l | tr -d ' ') -OTHER_COUNT=$(awk '$2 != 200 && $2 != 409' "$RESULTS_FILE" | wc -l | tr -d ' ') - -echo -echo "==> Child responses:" -sort "$RESULTS_FILE" -echo -echo " successes (200): $SUCCESS_COUNT" -echo " conflicts (409): $CONFLICT_COUNT" -echo " other: $OTHER_COUNT" -rm -f "$RESULTS_FILE" - -# 4. Refetch project; count how many targeted services are enabled. -PROJECT_JSON=$(get_project_state) -ENABLED_COUNT=0 -for svc in "${SERVICES[@]}"; do - # Capitalize first letter to form serviceStatusFor key. - Cap="$(echo "$svc" | awk '{print toupper(substr($1,1,1)) substr($1,2)}')" - val=$(echo "$PROJECT_JSON" | sed -nE "s/.*\"serviceStatusFor${Cap}\":[[:space:]]*(true|false).*/\1/p" | head -n1) - if [ "$val" = "true" ]; then - ENABLED_COUNT=$((ENABLED_COUNT + 1)) - fi -done - -echo " enabled in project state: $ENABLED_COUNT" -echo - -# 5. Verdict. -if [ "$SUCCESS_COUNT" -eq "$ENABLED_COUNT" ]; then - echo "PASS: every successful toggle persisted (no lost updates)." - EXIT=0 -else - echo "FAIL: lost updates detected. successes=$SUCCESS_COUNT enabled=$ENABLED_COUNT" - echo " Locking is either disabled or not effective on this endpoint." - EXIT=1 -fi - -# 6. Cleanup — re-enable all targeted services. -echo -echo "==> Cleanup: re-enabling ${SERVICES[*]}" -for svc in "${SERVICES[@]}"; do - toggle_service "$svc" true >/dev/null || true -done - -rm -f /tmp/lock-smoke-body.$$ -exit "$EXIT" diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/AuthMethods/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/AuthMethods/Update.php index 1ce36c8577..0d1cd83203 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/AuthMethods/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/AuthMethods/Update.php @@ -60,7 +60,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -71,22 +70,17 @@ class Update extends Action Database $dbForPlatform, Document $project, Authorization $authorization, - Event $queueForEvents, - callable $distributedLockOrFail, + Event $queueForEvents ): void { $auth = Config::getParam('auth')[$methodId] ?? []; $authKey = $auth['key'] ?? ''; - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $authKey, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); + $auths[$authKey] = $enabled; - $auths = $project->getAttribute('auths', []); - $auths[$authKey] = $enabled; - - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ + 'auths' => $auths, + ]))); $queueForEvents->setParam('methodId', $methodId); diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Create.php b/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Create.php index 8ab0653df3..f4002c60ef 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Create.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Create.php @@ -59,7 +59,6 @@ class Create extends Action ->inject('project') ->inject('dbForPlatform') ->inject('authorization') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -71,8 +70,21 @@ class Create extends Action Document $project, Database $dbForPlatform, Authorization $authorization, - callable $distributedLockOrFail, ) { + $auths = $project->getAttribute('auths', []); + + $mockNumbers = $auths['mockNumbers'] ?? []; + + if (\count($mockNumbers) >= APP_LIMIT_COUNT) { + throw new Exception(Exception::MOCK_NUMBER_LIMIT_EXCEEDED); + } + + foreach ($mockNumbers as $mockNumber) { + if ($mockNumber['phone'] === $number) { + throw new Exception(Exception::MOCK_NUMBER_ALREADY_EXISTS); + } + } + // Set to now date $mockNumber = [ 'phone' => $number, @@ -81,29 +93,14 @@ class Create extends Action '$updatedAt' => DateTime::now(), ]; - $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $number, $mockNumber, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $mockNumbers[] = $mockNumber; + $auths['mockNumbers'] = $mockNumbers; - $auths = $project->getAttribute('auths', []); - $mockNumbers = $auths['mockNumbers'] ?? []; + $updates = new Document([ + 'auths' => $auths, + ]); - if (\count($mockNumbers) >= APP_LIMIT_COUNT) { - throw new Exception(Exception::MOCK_NUMBER_LIMIT_EXCEEDED); - } - - foreach ($mockNumbers as $existing) { - if ($existing['phone'] === $number) { - throw new Exception(Exception::MOCK_NUMBER_ALREADY_EXISTS); - } - } - - $mockNumbers[] = $mockNumber; - $auths['mockNumbers'] = $mockNumbers; - - $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents->setParam('number', $number); diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Delete.php b/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Delete.php index bbc534fdf8..0fb23e1764 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Delete.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Delete.php @@ -58,7 +58,6 @@ class Delete extends Action ->inject('project') ->inject('dbForPlatform') ->inject('authorization') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -69,33 +68,33 @@ class Delete extends Action Document $project, Database $dbForPlatform, Authorization $authorization, - callable $distributedLockOrFail, ) { - $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $number, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); - $auths = $project->getAttribute('auths', []); - $mockNumbers = $auths['mockNumbers'] ?? []; + $mockNumbers = $auths['mockNumbers'] ?? []; - $mockNumberIndex = null; - foreach ($mockNumbers as $index => $mock) { - if ($mock['phone'] === $number) { - $mockNumberIndex = $index; - break; - } + $mockNumberIndex = null; + foreach ($mockNumbers as $index => $mock) { + if ($mock['phone'] === $number) { + $mockNumberIndex = $index; + break; } + } - if (\is_null($mockNumberIndex)) { - throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND); - } + if (\is_null($mockNumberIndex)) { + throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND); + } - unset($mockNumbers[$mockNumberIndex]); - $auths['mockNumbers'] = array_values($mockNumbers); + unset($mockNumbers[$mockNumberIndex]); + $mockNumbers = array_values($mockNumbers); - $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $auths['mockNumbers'] = $mockNumbers; + + $updates = new Document([ + 'auths' => $auths, + ]); + + $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents->setParam('number', $number); diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Update.php index a492d7c7bd..48b90a1b97 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/MockPhone/Update.php @@ -59,7 +59,6 @@ class Update extends Action ->inject('project') ->inject('dbForPlatform') ->inject('authorization') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -71,41 +70,38 @@ class Update extends Action Document $project, Database $dbForPlatform, Authorization $authorization, - callable $distributedLockOrFail, ) { - $mockNumber = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $number, $otp, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); - $auths = $project->getAttribute('auths', []); - $mockNumbers = $auths['mockNumbers'] ?? []; + $mockNumbers = $auths['mockNumbers'] ?? []; - $mockNumberIndex = null; - foreach ($mockNumbers as $index => $mock) { - if ($mock['phone'] === $number) { - $mockNumberIndex = $index; - break; - } + $mockNumberIndex = null; + foreach ($mockNumbers as $index => $mock) { + if ($mock['phone'] === $number) { + $mockNumberIndex = $index; + break; } + } - if (\is_null($mockNumberIndex)) { - throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND); - } + if (\is_null($mockNumberIndex)) { + throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND); + } - $mockNumbers[$mockNumberIndex]['otp'] = $otp; - $mockNumbers[$mockNumberIndex]['$updatedAt'] = DateTime::now(); - $auths['mockNumbers'] = $mockNumbers; + $mockNumbers[$mockNumberIndex]['otp'] = $otp; + $mockNumbers[$mockNumberIndex]['$updatedAt'] = DateTime::now(); - $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); + $auths['mockNumbers'] = $mockNumbers; - return $mockNumbers[$mockNumberIndex]; - }); + $updates = new Document([ + 'auths' => $auths, + ]); + + $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents->setParam('number', $number); $response ->setStatusCode(Response::STATUS_CODE_OK) - ->dynamic(new Document($mockNumber), Response::MODEL_MOCK_NUMBER); + ->dynamic(new Document($mockNumbers[$mockNumberIndex]), Response::MODEL_MOCK_NUMBER); } } diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/MembershipPrivacy/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/MembershipPrivacy/Update.php index a912ee48a6..c947ff225a 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/MembershipPrivacy/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/MembershipPrivacy/Update.php @@ -60,7 +60,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -75,33 +74,30 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $userId, $userEmail, $userPhone, $userName, $userMFA, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); - $auths = $project->getAttribute('auths', []); + if ($userId !== null) { + $auths['membershipsUserId'] = $userId; + } + if ($userEmail !== null) { + $auths['membershipsUserEmail'] = $userEmail; + } + if ($userPhone !== null) { + $auths['membershipsUserPhone'] = $userPhone; + } + if ($userName !== null) { + $auths['membershipsUserName'] = $userName; + } + if ($userMFA !== null) { + $auths['membershipsMfa'] = $userMFA; + } - if ($userId !== null) { - $auths['membershipsUserId'] = $userId; - } - if ($userEmail !== null) { - $auths['membershipsUserEmail'] = $userEmail; - } - if ($userPhone !== null) { - $auths['membershipsUserPhone'] = $userPhone; - } - if ($userName !== null) { - $auths['membershipsUserName'] = $userName; - } - if ($userMFA !== null) { - $auths['membershipsMfa'] = $userMFA; - } + $updates = new Document([ + 'auths' => $auths, + ]); - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordDictionary/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordDictionary/Update.php index 9cb310257e..e2c678abb6 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordDictionary/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordDictionary/Update.php @@ -56,7 +56,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -67,18 +66,15 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); + $auths['passwordDictionary'] = $enabled; - $auths = $project->getAttribute('auths', []); - $auths['passwordDictionary'] = $enabled; + $updates = new Document([ + 'auths' => $auths, + ]); - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordHistory/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordHistory/Update.php index cfff58c8f0..a8ae81caff 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordHistory/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordHistory/Update.php @@ -59,7 +59,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -70,18 +69,20 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $total, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); - $auths = $project->getAttribute('auths', []); - $auths['passwordHistory'] = \is_null($total) ? 0 : $total; + if (\is_null($total)) { + $auths['passwordHistory'] = 0; + } else { + $auths['passwordHistory'] = $total; + } - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $updates = new Document([ + 'auths' => $auths, + ]); + + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordPersonalData/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordPersonalData/Update.php index 580b04754b..9db7cf0549 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordPersonalData/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/PasswordPersonalData/Update.php @@ -57,7 +57,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -68,18 +67,15 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); + $auths['personalDataCheck'] = $enabled; - $auths = $project->getAttribute('auths', []); - $auths['personalDataCheck'] = $enabled; + $updates = new Document([ + 'auths' => $auths, + ]); - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionAlert/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionAlert/Update.php index 8f9f0a4c48..22b7a44b04 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionAlert/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionAlert/Update.php @@ -56,7 +56,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -67,18 +66,15 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); + $auths['sessionAlerts'] = $enabled; - $auths = $project->getAttribute('auths', []); - $auths['sessionAlerts'] = $enabled; + $updates = new Document([ + 'auths' => $auths, + ]); - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionDuration/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionDuration/Update.php index c8111b5563..ba72c93a6f 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionDuration/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionDuration/Update.php @@ -56,7 +56,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -67,18 +66,15 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $duration, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); + $auths['duration'] = $duration; - $auths = $project->getAttribute('auths', []); - $auths['duration'] = $duration; + $updates = new Document([ + 'auths' => $auths, + ]); - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionInvalidation/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionInvalidation/Update.php index e10c0d4081..8f8a959959 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionInvalidation/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionInvalidation/Update.php @@ -56,7 +56,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -67,18 +66,15 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); + $auths['invalidateSessions'] = $enabled; - $auths = $project->getAttribute('auths', []); - $auths['invalidateSessions'] = $enabled; + $updates = new Document([ + 'auths' => $auths, + ]); - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionLimit/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionLimit/Update.php index b111534430..382ed6f0d9 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionLimit/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/SessionLimit/Update.php @@ -57,7 +57,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -68,18 +67,20 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $total, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); - $auths = $project->getAttribute('auths', []); - $auths['maxSessions'] = \is_null($total) ? 0 : $total; + if (\is_null($total)) { + $auths['maxSessions'] = 0; + } else { + $auths['maxSessions'] = $total; + } - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $updates = new Document([ + 'auths' => $auths, + ]); + + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/UserLimit/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/UserLimit/Update.php index c59a17b61f..9129b81250 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/UserLimit/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Policies/UserLimit/Update.php @@ -57,7 +57,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -68,18 +67,20 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $total, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $auths = $project->getAttribute('auths', []); - $auths = $project->getAttribute('auths', []); - $auths['limit'] = \is_null($total) ? 0 : $total; + if (\is_null($total)) { + $auths['limit'] = 0; + } else { + $auths['limit'] = $total; + } - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'auths' => $auths, - ]))); - }); + $updates = new Document([ + 'auths' => $auths, + ]); + + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents ->setParam('projectId', $project->getId()) diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Protocols/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Protocols/Update.php index 0f3174b143..ad5691c1e0 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Protocols/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Protocols/Update.php @@ -60,7 +60,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -72,18 +71,13 @@ class Update extends Action Document $project, Authorization $authorization, Event $queueForEvents, - callable $distributedLockOrFail, ): void { - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $protocolId, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $protocols = $project->getAttribute('apis', []); + $protocols[$protocolId] = $enabled; - $protocols = $project->getAttribute('apis', []); - $protocols[$protocolId] = $enabled; - - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'apis' => $protocols, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ + 'apis' => $protocols, + ]))); $queueForEvents->setParam('protocolId', $protocolId); diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/SMTP/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/SMTP/Update.php index b8e3c6e09f..97e723f52c 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/SMTP/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/SMTP/Update.php @@ -72,7 +72,6 @@ class Update extends Action ->inject('dbForPlatform') ->inject('project') ->inject('authorization') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -91,101 +90,83 @@ class Update extends Action Response $response, Database $dbForPlatform, Document $project, - Authorization $authorization, - callable $distributedLockOrFail, + Authorization $authorization ): void { - $inputs = [ - 'host' => $host, - 'port' => $port, - 'username' => $username, - 'password' => $password, - 'senderEmail' => $senderEmail, - 'senderName' => $senderName, - 'replyToEmail' => $replyToEmail, - 'replyToName' => $replyToName, - 'secure' => $secure, - 'enabled' => $enabled, - ]; + // Fetch current configuration + $smtp = $project->getAttribute('smtp', []); - // The SMTP test (PHPMailer SmtpConnect with Timeout=5) runs inside the - // lock so two concurrent SMTP updates don't validate against the same - // baseline and overwrite each other's secrets. The 10s default lock - // TTL covers the worst-case 5s connection probe with margin. - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $inputs, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + // Apply changes + $keys = ['host', 'port', 'username', 'password', 'senderEmail', 'senderName', 'replyToEmail', 'replyToName', 'secure', 'enabled']; + foreach ($keys as $key) { + if (!\is_null(${$key})) { + $smtp[$key] = ${$key}; + } + } - // Fetch current configuration - $smtp = $project->getAttribute('smtp', []); + // Backwards compatibility + $smtp['replyToEmail'] = $smtp['replyToEmail'] ?? $smtp['replyTo'] ?? ''; - // Apply changes - foreach ($inputs as $key => $value) { - if (!\is_null($value)) { - $smtp[$key] = $value; + if (($smtp['enabled'] ?? false) === true) { + // Ensure required fields are set + $requiredKeys = ['host', 'port', 'senderEmail']; + foreach ($requiredKeys as $key) { + if (empty($smtp[$key])) { + throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.'); } } + } - // Backwards compatibility - $smtp['replyToEmail'] = $smtp['replyToEmail'] ?? $smtp['replyTo'] ?? ''; + // Validate SMTP credentials + // Validate when the caller is explicitly enabling or hasn't expressed a preference + // (so a credentials-only PATCH can auto-enable). Skip only when the caller is + // explicitly keeping/turning SMTP off. + if (\is_null($enabled) || $enabled === true) { + $mail = new PHPMailer(true); + $mail->isSMTP(); - if (($smtp['enabled'] ?? false) === true) { - // Ensure required fields are set - $requiredKeys = ['host', 'port', 'senderEmail']; - foreach ($requiredKeys as $key) { - if (empty($smtp[$key])) { - throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.'); - } - } + $mail->Host = $smtp['host'] ?? ''; + $mail->Port = $smtp['port'] ?? ''; + $mail->SMTPSecure = $smtp['secure'] ?? ''; + $mail->setFrom($smtp['senderEmail'], $smtp['senderName'] ?? ''); + + if (!empty($smtp['username'] ?? '')) { + $mail->SMTPAuth = true; + $mail->Username = $smtp['username']; + $mail->Password = $smtp['password'] ?? ''; } - // Validate SMTP credentials - // Validate when the caller is explicitly enabling or hasn't expressed a preference - // (so a credentials-only PATCH can auto-enable). Skip only when the caller is - // explicitly keeping/turning SMTP off. - if (\is_null($enabled) || $enabled === true) { - $mail = new PHPMailer(true); - $mail->isSMTP(); - - $mail->Host = $smtp['host'] ?? ''; - $mail->Port = $smtp['port'] ?? ''; - $mail->SMTPSecure = $smtp['secure'] ?? ''; - $mail->setFrom($smtp['senderEmail'], $smtp['senderName'] ?? ''); - - if (!empty($smtp['username'] ?? '')) { - $mail->SMTPAuth = true; - $mail->Username = $smtp['username']; - $mail->Password = $smtp['password'] ?? ''; - } - - if (!empty($smtp['replyToEmail'] ?? '')) { - $mail->addReplyTo($smtp['replyToEmail'], $smtp['replyToName'] ?? ''); - } - - $mail->SMTPAutoTLS = false; - $mail->Timeout = 5; - - try { - $valid = $mail->SmtpConnect(); - - if (!$valid) { - throw new \Exception('Connection is not valid.'); - } - - // Auto-enable if configuration is valid - // Dont do this if specifically request to mark disabled - if (\is_null($enabled)) { - $smtp['enabled'] = true; - } - } catch (Throwable $error) { - if (($smtp['enabled'] ?? null) === true) { - throw new Exception(Exception::PROJECT_SMTP_CONFIG_INVALID, $error->getMessage()); - } - } + if (!empty($smtp['replyToEmail'] ?? '')) { + $mail->addReplyTo($smtp['replyToEmail'], $smtp['replyToName'] ?? ''); } - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'smtp' => $smtp, - ]))); - }); + $mail->SMTPAutoTLS = false; + $mail->Timeout = 5; + + try { + $valid = $mail->SmtpConnect(); + + if (!$valid) { + throw new \Exception('Connection is not valid.'); + } + + // Auto-enable if configuration is valid + // Dont do this if specifically request to mark disabled + if (\is_null($enabled)) { + $smtp['enabled'] = true; + } + } catch (Throwable $error) { + if (($smtp['enabled'] ?? null) === true) { + throw new Exception(Exception::PROJECT_SMTP_CONFIG_INVALID, $error->getMessage()); + } + } + } + + // Save configuration + $updates = new Document([ + 'smtp' => $smtp, + ]); + + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $response->dynamic($project, Response::MODEL_PROJECT); } diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php index 1126da8897..7aab6f5ad0 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php @@ -60,7 +60,6 @@ class Update extends Action ->inject('project') ->inject('authorization') ->inject('queueForEvents') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -71,26 +70,14 @@ class Update extends Action Database $dbForPlatform, Document $project, Authorization $authorization, - Event $queueForEvents, - callable $distributedLockOrFail, + Event $queueForEvents ): void { - // The services map is a JSON object on the project document. Two - // concurrent service toggles read the same baseline, each set their - // own key, and the second updateDocument() overwrites the first — - // silent lost-update on a sparse write. Lock the project doc for - // the read-modify-write window; re-read inside the lock so the - // baseline reflects any update that landed between request init - // and lock acquisition. - $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $serviceId, $enabled, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + $services = $project->getAttribute('services', []); + $services[$serviceId] = $enabled; - $services = $project->getAttribute('services', []); - $services[$serviceId] = $enabled; - - return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'services' => $services, - ]))); - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ + 'services' => $services, + ]))); $queueForEvents->setParam('serviceId', $serviceId); diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Templates/Email/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Templates/Email/Update.php index ed1f7d8f7f..ef93abf683 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Templates/Email/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Templates/Email/Update.php @@ -69,7 +69,6 @@ class Update extends Action ->inject('dbForPlatform') ->inject('authorization') ->inject('project') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } @@ -87,61 +86,47 @@ class Update extends Action Database $dbForPlatform, Authorization $authorization, Document $project, - callable $distributedLockOrFail, ) { $locale = $locale ?: System::getEnv('_APP_LOCALE', 'en'); - $inputs = [ - 'senderName' => $senderName, - 'senderEmail' => $senderEmail, - 'replyToEmail' => $replyToEmail, - 'replyToName' => $replyToName, - 'message' => $message, - 'subject' => $subject, - ]; + // Prevent template update if custom SMTP is not configured + $smtp = $project->getAttribute('smtp', []); + if (($smtp['enabled'] ?? false) !== true) { + throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'SMTP must be enabled on the project to configure custom email templates.'); + } - $template = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $templateId, $locale, $inputs, $dbForPlatform, $authorization) { - $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); + // Fetch current configuration + $templates = $project->getAttribute('templates', []); + $template = $templates['email.' . $templateId . '-' . $locale] ?? []; - // Prevent template update if custom SMTP is not configured - $smtp = $project->getAttribute('smtp', []); - if (($smtp['enabled'] ?? false) !== true) { - throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'SMTP must be enabled on the project to configure custom email templates.'); + // Apply changes + $keys = ['senderName', 'senderEmail', 'replyToEmail', 'replyToName', 'message', 'subject']; + foreach ($keys as $key) { + if (!\is_null(${$key})) { + $template[$key] = ${$key}; } + } - // Fetch current configuration - $templates = $project->getAttribute('templates', []); - $template = $templates['email.' . $templateId . '-' . $locale] ?? []; + // Backwards compatibility + if (!\is_null($template['replyTo'] ?? null)) { + $template['replyToEmail'] = $template['replyToEmail'] ?? $template['replyTo'] ?? ''; + } - // Apply changes - foreach ($inputs as $key => $value) { - if (!\is_null($value)) { - $template[$key] = $value; - } + // Ensure required fields are set + $requiredKeys = ['subject', 'message']; + foreach ($requiredKeys as $key) { + if (empty($template[$key])) { + throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.'); } + } - // Backwards compatibility - if (!\is_null($template['replyTo'] ?? null)) { - $template['replyToEmail'] = $template['replyToEmail'] ?? $template['replyTo'] ?? ''; - } + // Save configuration + $templates['email.' . $templateId . '-' . $locale] = $template; + $updates = new Document([ + 'templates' => $templates, + ]); - // Ensure required fields are set - $requiredKeys = ['subject', 'message']; - foreach ($requiredKeys as $key) { - if (empty($template[$key])) { - throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.'); - } - } - - // Save configuration - $templates['email.' . $templateId . '-' . $locale] = $template; - - $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'templates' => $templates, - ]))); - - return $template; - }); + $project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates)); $queueForEvents->setParam('templateId', $templateId); diff --git a/src/Appwrite/Platform/Modules/Projects/Http/Projects/Team/Update.php b/src/Appwrite/Platform/Modules/Projects/Http/Projects/Team/Update.php index 25286a2227..ab92d1a15f 100644 --- a/src/Appwrite/Platform/Modules/Projects/Http/Projects/Team/Update.php +++ b/src/Appwrite/Platform/Modules/Projects/Http/Projects/Team/Update.php @@ -55,37 +55,29 @@ class Update extends Action ->param('teamId', '', new UID(), 'Team ID of the team to transfer project to.') ->inject('response') ->inject('dbForPlatform') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } - public function action(string $projectId, string $teamId, Response $response, Database $dbForPlatform, callable $distributedLockOrFail) + public function action(string $projectId, string $teamId, Response $response, Database $dbForPlatform) { - // Lock around the project doc RMW. Cascade fan-out to installations, - // repositories and vcsComments runs after the lock is released — - // those write to separate collections, not the project doc. - [$project, $permissions] = $distributedLockOrFail("lock:platform:projects:{$projectId}", function () use ($projectId, $teamId, $dbForPlatform) { - $project = $dbForPlatform->getDocument('projects', $projectId); - $team = $dbForPlatform->getDocument('teams', $teamId); + $project = $dbForPlatform->getDocument('projects', $projectId); + $team = $dbForPlatform->getDocument('teams', $teamId); - if ($project->isEmpty()) { - throw new Exception(Exception::PROJECT_NOT_FOUND); - } + if ($project->isEmpty()) { + throw new Exception(Exception::PROJECT_NOT_FOUND); + } - if ($team->isEmpty()) { - throw new Exception(Exception::TEAM_NOT_FOUND); - } + if ($team->isEmpty()) { + throw new Exception(Exception::TEAM_NOT_FOUND); + } - $permissions = $this->getPermissions($teamId, $projectId); + $permissions = $this->getPermissions($teamId, $projectId); - $project = $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ - 'teamId' => $teamId, - 'teamInternalId' => $team->getSequence(), - '$permissions' => $permissions, - ])); - - return [$project, $permissions]; - }); + $project = $dbForPlatform->updateDocument('projects', $project->getId(), new Document([ + 'teamId' => $teamId, + 'teamInternalId' => $team->getSequence(), + '$permissions' => $permissions, + ])); $installations = $dbForPlatform->find('installations', [ Query::equal('projectInternalId', [$project->getSequence()]), diff --git a/src/Appwrite/Platform/Modules/Projects/Http/Projects/Update.php b/src/Appwrite/Platform/Modules/Projects/Http/Projects/Update.php index 4b79b37bfa..29c26b33ea 100644 --- a/src/Appwrite/Platform/Modules/Projects/Http/Projects/Update.php +++ b/src/Appwrite/Platform/Modules/Projects/Http/Projects/Update.php @@ -65,36 +65,29 @@ class Update extends Action ->param('legalTaxId', '', new Text(256), 'Project legal tax ID. Max length: 256 chars.', true) ->inject('response') ->inject('dbForPlatform') - ->inject('distributedLockOrFail') ->callback($this->action(...)); } - public function action(string $projectId, string $name, string $description, string $logo, string $url, string $legalName, string $legalCountry, string $legalState, string $legalCity, string $legalAddress, string $legalTaxId, Response $response, Database $dbForPlatform, callable $distributedLockOrFail) + public function action(string $projectId, string $name, string $description, string $logo, string $url, string $legalName, string $legalCountry, string $legalState, string $legalCity, string $legalAddress, string $legalTaxId, Response $response, Database $dbForPlatform) { - // Re-fetch and write the full project doc inside the lock. This is the - // worst RMW window in the projects API — the endpoint passes the entire - // document back to updateDocument(), so a concurrent write to *any* - // attribute (services, auths, smtp, ...) would be silently overwritten. - $project = $distributedLockOrFail("lock:platform:projects:{$projectId}", function () use ($projectId, $name, $description, $logo, $url, $legalName, $legalCountry, $legalState, $legalCity, $legalAddress, $legalTaxId, $dbForPlatform) { - $project = $dbForPlatform->getDocument('projects', $projectId); + $project = $dbForPlatform->getDocument('projects', $projectId); - if ($project->isEmpty()) { - throw new Exception(Exception::PROJECT_NOT_FOUND); - } + if ($project->isEmpty()) { + throw new Exception(Exception::PROJECT_NOT_FOUND); + } - return $dbForPlatform->updateDocument('projects', $project->getId(), $project - ->setAttribute('name', $name) - ->setAttribute('description', $description) - ->setAttribute('logo', $logo) - ->setAttribute('url', $url) - ->setAttribute('legalName', $legalName) - ->setAttribute('legalCountry', $legalCountry) - ->setAttribute('legalState', $legalState) - ->setAttribute('legalCity', $legalCity) - ->setAttribute('legalAddress', $legalAddress) - ->setAttribute('legalTaxId', $legalTaxId) - ->setAttribute('search', implode(' ', [$projectId, $name]))); - }); + $project = $dbForPlatform->updateDocument('projects', $project->getId(), $project + ->setAttribute('name', $name) + ->setAttribute('description', $description) + ->setAttribute('logo', $logo) + ->setAttribute('url', $url) + ->setAttribute('legalName', $legalName) + ->setAttribute('legalCountry', $legalCountry) + ->setAttribute('legalState', $legalState) + ->setAttribute('legalCity', $legalCity) + ->setAttribute('legalAddress', $legalAddress) + ->setAttribute('legalTaxId', $legalTaxId) + ->setAttribute('search', implode(' ', [$projectId, $name]))); $response->dynamic($project, Response::MODEL_PROJECT); } diff --git a/tests/e2e/Client.php b/tests/e2e/Client.php index 84c937d2d1..4358058fe3 100644 --- a/tests/e2e/Client.php +++ b/tests/e2e/Client.php @@ -211,13 +211,7 @@ class Client } } - // CURLOPT_PATH_AS_IS isn't supported by Swoole's emulated cURL when the - // SWOOLE_HOOK_CURL coroutine hook is active. Skip it in that case so - // tests that need real parallel HTTP (Swoole\Coroutine\run + cURL hook) - // don't fatal-error here. Native (non-hooked) cURL keeps the option. - if (! \extension_loaded('swoole') || ! (\Swoole\Runtime::getHookFlags() & SWOOLE_HOOK_CURL)) { - curl_setopt($ch, CURLOPT_PATH_AS_IS, 1); - } + curl_setopt($ch, CURLOPT_PATH_AS_IS, 1); curl_setopt($ch, CURLOPT_CUSTOMREQUEST, $method); curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); curl_setopt($ch, CURLOPT_FOLLOWLOCATION, $followRedirects); @@ -243,12 +237,9 @@ class Client if ($method === self::METHOD_HEAD) { curl_setopt($ch, CURLOPT_NOBODY, true); // This is crucial for HEAD requests curl_setopt($ch, CURLOPT_HEADER, false); + } else { + curl_setopt($ch, CURLOPT_NOBODY, false); } - // Note: explicit CURLOPT_NOBODY=false on non-HEAD requests is redundant - // (false is cURL's default) and actively breaks Swoole's emulated cURL - // on PATCH-with-body — Swoole strips the body and the request reaches - // the server as a method-without-body, hitting the framework's 404 - // fallback. Just skip the redundant set. if ($method != self::METHOD_GET && $method != self::METHOD_HEAD) { curl_setopt($ch, CURLOPT_POSTFIELDS, $query); diff --git a/tests/e2e/Services/Project/ServicesBase.php b/tests/e2e/Services/Project/ServicesBase.php index c8c7bd173f..b5f94f8181 100644 --- a/tests/e2e/Services/Project/ServicesBase.php +++ b/tests/e2e/Services/Project/ServicesBase.php @@ -266,94 +266,6 @@ trait ServicesBase $this->assertSame(true, $response['body']['serviceStatusForTeams']); } - /** - * Concurrency test for the distributed-lock pilot on `updateProjectService`. - * - * Without locking, two concurrent toggles to different services on the - * same project both read the same baseline `services` map, each set their - * own key, and the second sparse `updateDocument()` overwrites the first - * — silent lost-update. - * - * The test fires N parallel PATCH calls via Swoole coroutines (with the - * SWOOLE_HOOK_CURL runtime hook enabled so the test Client's cURL calls - * yield to the scheduler). After all coroutines complete the project is - * refetched and the count of enabled services is compared against the - * count of HTTP 200 responses. - * - * To verify the test catches the bug: - * - `_APP_LOCKING_ENABLED=enabled` → must pass - * - `_APP_LOCKING_ENABLED=disabled` → must FAIL (lost updates) - */ - public function testConcurrentTogglesAllPersist(): void - { - $services = ['teams', 'storage', 'functions', 'sites', 'messaging']; - - // Baseline: disable everything so the toggle direction is unambiguous. - // Done outside the coroutine context so it stays sequential and the - // resulting project state is a clean known-zero before the race. - foreach ($services as $service) { - $this->updateServiceStatus($service, false); - } - - // Enable Swoole's cURL hook so the test Client's HTTP calls yield - // to the scheduler and the foreach below actually runs in parallel. - // Without this, coroutines serialize on cURL and the negative-case - // run (locking disabled) would falsely pass. - \Swoole\Runtime::enableCoroutine(\SWOOLE_HOOK_CURL); - - $results = []; - try { - \Swoole\Coroutine\run(function () use ($services, &$results): void { - foreach ($services as $service) { - \Swoole\Coroutine::create(function () use ($service, &$results): void { - $response = $this->updateServiceStatus($service, true); - $results[$service] = $response['headers']['status-code']; - }); - } - }); - } finally { - // SWOOLE_HOOK_NONE isn't defined in some Swoole builds; pass 0 - // (the integer value of "no hooks") to disable any hooks this - // test enabled so subsequent tests run with native cURL. - \Swoole\Runtime::enableCoroutine(0); - } - - $successCount = count(array_filter($results, fn ($code) => $code === 200)); - - // Refetch the project and count which of the targeted services ended up - // enabled. The endpoint returns serviceStatusFor{Service} keys. - $project = $this->client->call(Client::METHOD_GET, '/projects/' . $this->getProject()['$id'], [ - 'content-type' => 'application/json', - 'x-appwrite-project' => 'console', - 'cookie' => 'a_session_console=' . $this->getRoot()['session'], - ]); - - $enabledCount = 0; - foreach ($services as $service) { - $key = 'serviceStatusFor' . ucfirst($service); - if (($project['body'][$key] ?? false) === true) { - $enabledCount++; - } - } - - $this->assertGreaterThan(0, $successCount, 'At least one concurrent toggle should succeed'); - $this->assertSame( - $successCount, - $enabledCount, - sprintf( - 'Each successful concurrent toggle must persist. successCount=%d enabledCount=%d (lost-update detected — distributed lock not effective)', - $successCount, - $enabledCount, - ), - ); - - // Cleanup: leave all targeted services enabled so subsequent tests - // see a known-good baseline. - foreach ($services as $service) { - $this->updateServiceStatus($service, true); - } - } - // Helpers protected function updateServiceStatus(string $serviceId, bool $enabled, bool $authenticated = true): mixed